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.

Sign up for GitHub

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

Jump to bottom

[MAINT] update contributors #1964

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

[MAINT] update contributors #1964

wants to merge 21 commits into from
+1,158 −387

Conversation

Remi-Gau
Copy link
Collaborator

@Remi-Gau Remi-Gau commented Oct 20, 2024 •
edited
Loading

@Remi-Gau Remi-Gau added the exclude-from-changelog This item will not feature in the automatically generated changelog label Oct 20, 2024
@Remi-Gau Remi-Gau changed the title update contributors [MAINT] update contributors Oct 20, 2024
@codecov Codecov
Copy link

codecov bot commented Oct 20, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.87%. Comparing base (790c0fb) to head (58215d1).

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1964   +/-   ##
=======================================
  Coverage   86.87%   86.87%           
=======================================
  Files          16       16           
  Lines        1410     1410           
=======================================
  Hits         1225     1225           
  Misses        185      185

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Remi-Gau Remi-Gau marked this pull request as draft October 21, 2024 07:48
@Remi-Gau
Copy link
Collaborator Author

Remi-Gau commented Oct 21, 2024 •
edited
Loading

converting to draft:

  • check if some of the new contributors have a github account (I know some do)
  • need to add a few more contributors from some recently developed BIDS tools

@Remi-Gau
Copy link
Collaborator Author

TODO (but for future PRs):

  • add github usernames to contributors who have one
  • update affiliations (help to know where our contributors are from)
  • check people who have contributed to repos on BIDS and BIDS apps github organization, tools / converters repos...
  • check people mentioned on BIDS papers (that probably should be listed as contributors ?)

@Remi-Gau Remi-Gau marked this pull request as ready for review October 21, 2024 11:31
bendhouseart
bendhouseart approved these changes Oct 21, 2024
View reviewed changes
Copy link
Collaborator

@bendhouseart bendhouseart left a comment

Choose a reason for hiding this comment

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

lgtm

effigies
effigies reviewed Oct 24, 2024
View reviewed changes
src/appendices/contributors.md
@@ -55,13 +56,14 @@ If you contributed to the BIDS ecosystem and your name is not listed, please add
| Alexander Jones | 💻🐛 |
| Alexander L. Cohen | 🐛💻📖💬 |
| Alexander von Lautz | 📖 |
| Alexandre D'Astous | 📖 |
Copy link
Collaborator

Choose a reason for hiding this comment

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

@po09i I'm assuming you didn't remove yourself.

@Remi-Gau Do you know what happened here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hypothesis: Alexandre was added only to the text file here in https://github.com/bids-standard/bids-specification/pull/1690/files but never to any of the "metadata" files for contributors. Thus whenever regenerated -- disappeared? So needs to gain a record in .tributors and .all-contributorsrc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah I am pretty sure this it what happened: will fix

effigies
effigies reviewed Oct 24, 2024
View reviewed changes
.tributors
],
"email": "[email protected]",
"github": "mguaypaq",
"name": "mathieu_guay-paquet"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Surely:

Suggested change
"name": "mathieu_guay-paquet"
"name": "Mathieu Guay-Paquet"

yarikoptic
yarikoptic reviewed Oct 28, 2024
View reviewed changes
tools/add_contributors.py
@@ -46,7 +46,7 @@
# update with your GitHub username and path to a file with GitHub token
UPDATE_AVATARS = False
GH_USERNAME = "Remi-Gau"
TOKEN_FILE = None
TOKEN_FILE = "/home/remi/Documents/tokens/gh_user.txt"
Copy link
Collaborator

Choose a reason for hiding this comment

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

no absolute paths should be hardcoded IMHO

Copy link
Collaborator

Choose a reason for hiding this comment

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

should there be an env var or alike to define it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

probably unless the API calls done in here can be all handled be handled by the github CLI package (not sure)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@effigies effigies effigies left review comments

@yarikoptic yarikoptic yarikoptic left review comments

@bendhouseart bendhouseart bendhouseart approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
exclude-from-changelog This item will not feature in the automatically generated changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Remi-Gau @yarikoptic @effigies @bendhouseart