-
Notifications
You must be signed in to change notification settings - Fork 60
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
Update contributor and review data #527
Conversation
_data/packages.yml
Outdated
@@ -123,7 +162,7 @@ | |||
reviewers: | |||
- name: Tom Aldcroft | |||
github_username: taldcroft | |||
- name: Michele Delli Veneri | |||
- name: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above people's names are being randomly?? erased for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/MicheleDelliVeneri This user's name (as opposed to Chiara) isn't listed in the GitHub profile. BUT i thought we had added logic to handle that if the name was already in this file it should not overwrite it.
_data/packages.yml
Outdated
github_username: isabelizimm | ||
reviewers: | ||
- name: Phil Dong | ||
- name: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above, phils name is NOT in their profile so it's erasing likely because of that. instead we want to check the file and if a name is there, use the name from the file. this allows people to update their names here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: i'll open an issue based on this comment.
Update the Contributing file for this repo (or the readme) to explain that we should ONLY update a person's name in the contributors.yml file. If we try to update the packages.yml file with the name it will be overwritten. This is because our workflow treats the contributors.yml file as the singel source of truth and the single plae to manage people. The names listed here assume that we can gather metadata based on a contributor's GitHub username from the contributor file.
takeaway: update names in the contributors.yml file always (and document this well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay. I was not aware of this contributors.yml
file. Sorry to give you extra work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries! I think other people were also editing names in this packages.yml file. So, really, it's on me for not doing a better job documenting how this workflow works! I had forgotten myself how things were setup!
a lot of these items are fixed now however. AND if you want to submit a tiny pr to update names in the contributors.yml file OR if you want to open an issue about it that would work. pyosmeta first opens that file and it will NOT overwrite a manually added name. It will overwrite a missing name if it can find a name associated with the gh username
there are actually multiple issues and bugs that you've uncovered so it's all good to go through!!
@@ -355,7 +395,7 @@ | |||
- name: Patrick J. Roddy | |||
github_username: paddyroddy | |||
repository_link: https://github.com/astro-informatics/sleplet | |||
version_submitted: v1.4.4 | |||
version_submitted: '[v1.4.4](https://pypi.org/project/sleplet/1.4.4)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might require a conversation about how we want to populate version_submitted. people don't always have a tagged version but when they do and provide it, it's nice to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if you want to allow linking for this field, when I manually combed through the YAML, the way link is added was inconsistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup! i hear you. One way to fix this COULD be to add a note to the field that looks something like this:
version_submitted (v1.2.3)
version_submitted_link
Or alternatively we could just ask for a link to a release, tag or pypi version?
version_submitted (v1.4.4
we would want to talk to folks about this but clearly we are getting different responses for this field that we will need to populate with a validator!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm open to doing the thing that is most useful. on our end - adding a validation step that checks the version and tries to parse out any links might be a good first step.
A second step would be adding an example to the issue template to encourage consistency
@@ -368,7 +408,7 @@ | |||
github_username: magsol | |||
- name: Jakub Tomasz Gnyp | |||
github_username: gnypit | |||
archive: '[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.7268074.svg)](https://doi.org/10.5281/zenodo.7268074)' | |||
archive: '[10.5281/zenodo.7268074](https://doi.org/10.5281/zenodo.7268074)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here pyosmeta is making a decision about how doi's are processed. i opened an issue here about this
i think it's pretty easy for a user or maintainer or editor to copy a doi badge into the issue. and it renders nicely too. so that could be a way for us to go.
pyosmeta can them parse instances like this where the badge part is missing accordingly. the website can then render as needed based on the badge. right now it's parsing the badge into html.
we could decide to go the opposite way too but whatever we decide it needs to be consistent.
@@ -532,7 +569,7 @@ | |||
- name: Hassan Kibirige | |||
github_username: has2k1 | |||
archive: '[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.10776842.svg)](https://doi.org/10.5281/zenodo.10776842)' | |||
version_accepted: 0.11.0 | |||
version_accepted: '[0.11.0](https://github.com/vnmabus/rdata/releases/tag/0.11.0)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe for this we want two fields
version_accepted can just be a numeric value (not a list)
version_accepted_archive can be the github link to a release.
archive can refer to zenodo doi archive (and we can add a comment to our issue template about this)
joss: joss doi
_data/packages.yml
Outdated
@@ -617,15 +654,15 @@ | |||
- data-processing-munging | |||
editor: | |||
name: Emily Jane McTavish | |||
github_username: snacktavish | |||
github_username: snacktavish Emily Jane McTavish |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is another, different bug that recently emerged. why?
here the name is correct it's just released. we should have a validation step on the github username that it can only be one word. if there is a second word it should only include the first word. github usernames can't have spaces so this should be easily to validate in our model.
This pr highlights a bunch of bugs that we need to fix in our workflow. cc: @pllim who discovered and highlighted many of these in a recent set of issues and pr's. All of these need to be fixed in pyosmeta or else the bugs that have been manually fixed in the yml file will reappear in each new bot-trigger pr like this one!! we will tackle these issues in december and can. |
github_username: phildong | ||
- name: Iris Young | ||
github_username: irisdyoung | ||
- name: Nima Sarajpoor | ||
github_username: NimaSarajpoor | ||
archive: '[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.10864492.svg)](https://doi.org/10.5281/zenodo.10864492)' | ||
version_accepted: 8.4.0 | ||
version_accepted: 8.4.0 ([repo](https://github.com/caleb531/automata/releases/tag/v8.4.0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this has multiple values separated by commas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when something looks really odd like this - it's often good to start by checking the review. Here someone did some odd things in the review
my guess is our model just didn't know how to handle the additional information provided after the version number. I think we could set up a validator, however, to parse this into two fields in the future, as this version_accepted field is consistently filled out differently across issues!
it's really hard to parse human-entered free form text :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it can be separated into 2 fields
a. version (e.g., 8.4.0)
b. link to version release (e.g., https://pypi.org/project/automata-lib/8.4.0 )
Ideally the reviewer/EIC would fix the formatting before merge right, nipping such problems at the bud.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said, if you have a field for the PyPI project page (e.g., https://pypi.org/project/automata-lib/), then the link to version release becomes redundant and thus unnecessary. Astropy needs the PyPI project link anyway:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool. let's carry this conversation over to that issue. i want to resolve the structural issues that are breaking things first. adding a pypi item is related to version but it's also an independent thing because not everyone links to pypi for the version submitted or accepted!
let's get this build working first and then re-open the discussion of adding new fields. if we do that, we could then add a few notes around the format of fields to avoid the issues we just had to troubleshoot!!
@@ -425,9 +465,6 @@ | |||
github_username: vn-ki | |||
repository_link: https://github.com/sunpy/sunpy | |||
version_submitted: 5.0.1 | |||
sunpy: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't touch this in #520 but does seem weird it is removed here regardless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i may have added this manually back when i was playing with what partner community cards would look like. i wouldn't put it past me to do that and we probably could use blame to confirm. but i suspect I added it. Then I realized sunpy is associated with pyhc in some ways so we may want to have multiple affiliations for a single package in some cases.
github_username: JacksonBurns | ||
all_current_maintainers: | ||
- name: Kevin Spiekermann | ||
github_username: kspieks | ||
- name: Himaghna Bhattacharjee |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name was not listed on GitHub but I easily found it on the internet from other sources. I think we should be allowed to overwrite this field if real name is available elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. so I think I mentioned this above, but the rule is that we update names in one place: contributors.yml. That update will propagate to this file. that way we don't have to worry about updating in two places!!
We would use the same approach if this were a database. we'd have a person entry and then link them to various contribution types. :) Before we close this pr, let's make sure that the names that have been looked up are properly added to contributors.yml. they will then get updated in the packages.yml file in our next run of the bot! this should be working as i just tested it today!!
@@ -874,7 +911,7 @@ | |||
- name: | |||
github_username: khynder | |||
archive: '[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.8214655.svg)](https://doi.org/10.5281/zenodo.8214655)' | |||
version_accepted: v0.2.36 | |||
version_accepted: v [0.2.36](https://github.com/katoss/cardsort/releases/tag/v0.2.36) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example of linking inconsistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok i'll tackle the release items later (or i'll ask the person who's starting soon to work on it!). we may just want a validator for now to clean up any additional links.
_data/packages.yml
Outdated
forks_count: 4 | ||
documentation: https://www.phenopype.org | ||
documentation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is one way to "fix" a broken link... 😅
7246f86
to
38a7b1f
Compare
38a7b1f
to
9f21279
Compare
9f21279
to
2644d8c
Compare
This looks much better. i will merge but we are still working on the doi issues |
Automated changes by create-pull-request GitHub action