Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eslint updates #6

Merged
merged 12 commits into from
Aug 30, 2024
Merged

Conversation

jaydanielsencision
Copy link

Changes

  • migrate from coffeescript+grunt to pure nodeJs with node 20/LTS support and security updates
  • previous (generated) lib content retained.
  • added unit tests
  • many eslint updates.

"version": "0.1.2",
"homepage": "https://github.com/nunukim/node-xlsx-stream",
"version": "0.2.0",
"homepage": "https://github.com/BrandwatchLtd/node-xlsx-stream",
"author": {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to update the author info too? Seems out of date since the urls redirect oddly and doesn't apply to this codebase. Carried over from initial fork maybe?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the etiquette with forks is to update the author (to reflect who should be contacted w.r.t. the fork) but list the original author in contributors. The author field is optional - for our case might be best to leave it out.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking this might constituent as copyright attribution required by the MIT licence but it is, in fact, 100% not and the author copyright is in the LICENCE file anyway. Silly me.

@jaydanielsencision jaydanielsencision merged commit 21c87dd into master Aug 30, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants