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

Add automated contributors #70 #83

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

ASK-03
Copy link

@ASK-03 ASK-03 commented Mar 5, 2024

This PR solves Issue #70

This PR is an extension of PR #73 and preserves the commits of the previous contributor

Changes:

  • Add GITHUB_TOKEN using environment variable
  • Removed cryos/avogadro from repositories variable as it has been closed
  • Removed the code that updates credits.md as it is handled by Sphinx
  • Added code block to add full name of the contributor if its is not available then the username is used
  • Removed the sort as it was doing nothing.

@ASK-03
Copy link
Author

ASK-03 commented Mar 6, 2024

@ghutchis, I have done the necessary changes. Please have a look.

@ghutchis
Copy link
Member

ghutchis commented Mar 9, 2024

It looks good although the downside is that the rest of the contributors list is sorted, and the new contributors are added to the end. I think it would be better to insert in sorted order or re-sort after adding the new contributor.

@ASK-03
Copy link
Author

ASK-03 commented Mar 9, 2024

It looks good although the downside is that the rest of the contributors list is sorted, and the new contributors are added to the end. I think it would be better to insert in sorted order or re-sort after adding the new contributor.

Sure, I will add the changes. Could you please clarify if you prefer sort based on chronological order of first contribution or lexicographical sort would be suitable?

@ghutchis
Copy link
Member

ghutchis commented Mar 9, 2024

We have gone with lexicographical sort, thanks.

@ASK-03
Copy link
Author

ASK-03 commented Mar 9, 2024

@ghutchis. I have made the changes.

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