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

remove contributors from package.json in favour of AUTHORS #1774

Merged
merged 6 commits into from
Mar 23, 2020
Merged

Conversation

harrysarson
Copy link
Collaborator

The contributors section of our package.json is very long and manually updating can be a pain.
I took a leaf out of nodejs's book and copied their update-authors script 1.

The new list is generated based on commits to the develop branch of mathjs (it is actually based on the current branch when the script is run) so the list is slightly different from the one from our package.json.

@harrysarson
Copy link
Collaborator Author

Names present in package.json but not in commit logs

Most of these names have corresponding names in AUTHORS (e.g. Zach Zibrat is Zak Zibrat). Huseyn Guiliyev loses out though because he is noted as a contributor (due to participation in a number of issues I think) but hasn't committed to mathjs. I do not know how we can fix this though.

@harrysarson
Copy link
Collaborator Author

Names in AUTHORS that were not in package.json

Some of these are legit AUTHORS whom we forgot to add to the package.json. There are also some duplicates that I haven't managed to mail map out. @josdejong you are also part of this list!

@harrysarson harrysarson reopened this Mar 14, 2020
@josdejong
Copy link
Owner

Nice, I like it! It's indeed tricky not to forget to keep the list up to date.

Maybe we should run the update-authors script during npm run build so we can't forget running it?

The contributors section of our package.json is _very_ long and
manually updating can (?) be a pain.

I took a leaf out of nodejs's book and copied their update-authors
script [1].

[1]: https://github.com/nodejs/node/blob/master/tools/update-authors.js
@harrysarson
Copy link
Collaborator Author

harrysarson commented Mar 14, 2020

None of our other npm-scripts change files tracked by git. I worry about the following senario:

If a person "John Smith"

a) makes a change
b) runs npm run build-and-test
c) commits that change
d) gets that commit merged into master (via a pull request)

the next time someone runs npm run build-and-test then "John Smith" will suddenly appear in the AUTHORS file. Would an explicit instruction in how_to_publish.md suffice?

@josdejong
Copy link
Owner

That's a good point. OK can you describe this step in the how_to_publish.md then please?

@harrysarson
Copy link
Collaborator Author

See b2172a3

It looks like I typed the commit message out in a hurry, should probably say add instruction to how_to_publish instead. 🤷‍♂️

@josdejong josdejong merged commit 08feb7f into develop Mar 23, 2020
@josdejong josdejong deleted the authors branch March 23, 2020 18:57
@josdejong
Copy link
Owner

Thanks again Harry, I've merged your PR!

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.

2 participants