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

Adding tributors workflow and file #4789

Merged
merged 14 commits into from
Nov 25, 2020
Merged

Adding tributors workflow and file #4789

merged 14 commits into from
Nov 25, 2020

Conversation

vsoch
Copy link
Collaborator

@vsoch vsoch commented Jul 30, 2020

This pull request will add usage of the tributors library, which means that we can have an automated job to:

  • update the .zenodo.json with orcid ids (see TODOs)
  • update the .allcontributorsrc file to include new contributors!

This also means I ran commands locally to init the .allcontributors file (only needs to be done once) and used my own Orcid tokens locally to update the .zenodo. Note that the tool found several ids for some people's names/emails, and those would need to be manually figured out. Currently, I'm triggering the workflow on PR so we can test it.

TODOs:

  • ORCID: create the proper tokens

…lcontributors

Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

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

THANK YOU!

NB I tuned up a description to have a dedicated TODO for tokens...

I spotted some unicode characters not being encoded correctly while .zenodo.json being re-generated.
Also left some comments for CONTRIBUTING.md changes etc.

So, we need to provide a secret (ORCID_TOKEN and probably GITHUB_TOKEN)... I will try to do so now. I think I will "mint" a GITHUB token from datalad-tester user and invite "it" as a contributor with Write permissions to this repo

.zenodo.json Outdated Show resolved Hide resolved
.zenodo.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
.github/workflows/update-contributors.yml Show resolved Hide resolved
.github/workflows/update-contributors.yml Show resolved Hide resolved
.github/workflows/update-contributors.yml Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
.github/workflows/update-contributors.yml Show resolved Hide resolved
Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Collaborator Author

vsoch commented Jul 30, 2020

oh and @yarikoptic you don't need to create a GITHUB_TOKEN - its always there automatically if you need it.

Signed-off-by: vsoch <vsochat@stanford.edu>
README.md Outdated Show resolved Hide resolved
Signed-off-by: vsoch <vsochat@stanford.edu>
@yarikoptic
Copy link
Member

oh and @yarikoptic you don't need to create a GITHUB_TOKEN - its always there automatically if you need it.

OH! That makes so much sense (why github refuses to add one etc...) so I will just remove that token from datalad-tester now ;)

@yarikoptic
Copy link
Member

I have added both ORCID tokens into secrets here. Locally when I ran tributors update zenodo I get

the long list of possible De La Vega, Alejandro and a few candidates for C Lau, Vicky
(git)lena:~datalad/datalad[add/tributors]git
$> tributors update zenodo    
INFO:    zenodo:Updating .zenodo.json
INFO:    zenodo:Updating .tributors cache from .zenodo.json
WARNING:github:Found 43 results for De La Vega, Alejandro:

0000-0003-2138-5651
0000-0002-0464-0115
0000-0002-6029-8887
0000-0003-2231-4011
0000-0002-8427-1873
0000-0003-1071-4114
0000-0002-2687-0544
0000-0001-9325-3186
0000-0003-4102-1193
0000-0003-0313-818X
0000-0003-3258-1690
0000-0002-4941-1156
0000-0002-7465-7844
0000-0001-7035-8639
0000-0003-3755-3787
0000-0002-7237-6299
0000-0002-4773-1623
0000-0001-6215-3289
0000-0001-7683-8369
0000-0002-2785-5733
0000-0001-7084-5576
0000-0003-1199-0217
0000-0002-7242-318X
0000-0002-3527-0428
0000-0003-0231-1147
0000-0002-8100-3195
0000-0002-3587-8935
0000-0002-7380-374X
0000-0003-3579-9409
0000-0002-0503-5793
0000-0003-2585-0303
0000-0001-8191-3597
0000-0002-0606-3059
0000-0003-1744-9667
0000-0002-0971-8954
0000-0001-7813-9860
0000-0002-9243-082X
0000-0001-5676-6652
0000-0003-4137-4423
0000-0001-6031-8107
0000-0002-8423-3017
0000-0002-9060-0629
0000-0003-3532-7225
You should look these up and add the correct in your .tributors lookup
WARNING:github:Found 4 results for C Lau, Vicky:

0000-0001-9374-7098
0000-0001-9750-2514
0000-0003-3181-8561
0000-0003-0925-2012
You should look these up and add the correct in your .tributors lookup
WARNING:tributors:zenodo does not support updating from names.
although Alejandro does have orcid listed already
$> grep -2 Alejandro .zenodo.json
        {
            "affiliation": "University of Texas at Austin",
            "name": "De La Vega, Alejandro",
            "orcid": "0000-0001-9062-3778"
        },

.zenodo.json was rerendered (I use released tributors so unicode loss etc) but no other ORCID was added to anyone, or announced to have found (or not) candidates for... If I search manually by name for "Kyle Meyer" I do get many hits on orcid (@kyleam - do you have orcid id?), but I guess it might be that tributors searches by an email and doesn't find?

@vsoch
Copy link
Collaborator Author

vsoch commented Jul 30, 2020

Tributors tries email and name, and then shows a bunch of ids if it can't resolve to just one. @yarikoptic the search for the one that already exists is a bug, I'll fix this in the PR to also update unicode.

@vsoch
Copy link
Collaborator Author

vsoch commented Jul 30, 2020

okay the unicode and repeated check issues are fixed with con/tributors#44, I'll update the version here after releasing it.

@yarikoptic
Copy link
Member

well, I removed orcid from my and Michael Hanke's record in .zenodo.json, and got nothing for poor us.
Running in debug it shows that it uses github id as a given name (not sure if worthwhile - could lead to too many false positives)

$> tributors --log-level DEBUG update zenodo
DEBUG:urllib3.connectionpool:https://pub.orcid.org:443 "GET /v3.0/search/?q=given-names:yarikoptic+AND+family-name: HTTP/1.1" 500 None

and then by full name but seems incorrectly given/family name:

DEBUG:urllib3.connectionpool:https://pub.orcid.org:443 "GET /v3.0/search/?q=given-names:Halchenko,+AND+family-name:Yaroslav%20O. HTTP/1.1" 200 None

and also there is %20 from unsanitized input...

@vsoch
Copy link
Collaborator Author

vsoch commented Jul 30, 2020

@yarikoptic I'm not sure what you mean by the input, but feel free to open a PR to fix the bug that you see. Thanks!

Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Collaborator Author

vsoch commented Jul 30, 2020

doh, just added a bug. Give me a few minutes!

Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
- name: Checkout New Branch
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BRANCH_AGAINST: "maint"
Copy link
Member

Choose a reason for hiding this comment

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

I don't see relevant discussion, so will start a new one: why here we are not picking up the target branch PR is intended for, or current branch this workflow runs for ATM if not a PR? My point was: we have multiple branches against which people could submit a PR - e.g. it could be maint or master. I guess tributors should operate on that branch, not some hard coded maint or master.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be run after a contribution is merged (and thus final), which is reliably to master or maint.

Copy link
Member

Choose a reason for hiding this comment

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

exactly - "master or maint", so it should not be hardcoded to maint here -- it might arrive to master. isn't there some variable available already to use here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be master, actually, and a variable (if it were available) would be less clear to the reader/developer.

```

And then we can rely on the [GitHub action](.github/workflows/update-contributors.yml) to update contributors, and
look for new orcids as we did above.
Copy link
Member

Choose a reason for hiding this comment

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

Please clarify that it would send a PR, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, when a contribution is made (as in, merged into master) it would be triggered and open a PR.

Copy link
Member

Choose a reason for hiding this comment

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

so should be reflected in the text above?

CONTRIBUTING.md Outdated Show resolved Hide resolved
vsoch and others added 3 commits August 3, 2020 17:49
Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
@codecov
Copy link

codecov bot commented Aug 7, 2020

Codecov Report

Merging #4789 (3ea9e38) into master (477782d) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4789   +/-   ##
=======================================
  Coverage   60.20%   60.21%           
=======================================
  Files         130      130           
  Lines       19449    19449           
=======================================
+ Hits        11710    11711    +1     
+ Misses       7739     7738    -1     
Impacted Files Coverage Δ
datalad/support/gitrepo.py 88.57% <0.00%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 477782d...3ea9e38. Read the comment docs.

* origin/master: (533 commits)
  DOC: Remove mentioning of singularity containers in README.md
  install.py: Add "datalad" schema for installing Miniconda with datalad
  ENH: wtf --flavor short to show only datalad + dependencies
  DOC: common_cfg: Mark save.no-message=interactive as experimental
  Make benchmark workflow fail if a benchmark slowed down too much
  DOC: call_git_oneline: Drop non-existent parameter from docstring
  ENH(DOC): add supported Python version(s) badge and place badges one at a line in md
  BF(DOC): add plain Singularity (without tags)
  MNT: CHANGELOG.md: Fourth batch for 0.14.0
  RF: customremotes: Remove unused _get_key_path() method
  Test of the miniconda install scheme
  DOC: customremotes/archives: Drop stale comment
  BF: Handle annex-get JSON output to prevent protocol error
  BF: ora: Trigger SSH cleanup on failure to prevent stall
  BF: Make push try to initialize a fresh remote before copying
  BF: Do config reload after AnnexRepo.localsync()
  Use urllib instead of wget
  Fix bug
  Support installing Miniconda on macOS
  Use --batch option to conda-forge scheme in .travis.yml
  ...
…dd/tributors

* 'add/tributors' of git://github.com/vsoch/datalad:
  also updating zenodo.json
  updating to use con/tributors 0.0.19
@yarikoptic
Copy link
Member

To resolve conflicts, merged current master and vsoch's add/tributors (sorry for messy history)

export ORCID_ID=xxxxxxxxxx
export ORCID_SECRET=xxxxxxxxxxxxxxxxxx
export ORCID_TOKEN=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
export ORCID_REFRESH_TOKEN=xxxxxxxxxxxxxxxxxxxxxxxxx
Copy link
Member

Choose a reason for hiding this comment

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

I have some vague memory ORCID stuff isn't needed any longer, or only some of them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I think that is correct!

@yarikoptic
Copy link
Member

ok, I have removed mentioning of ORCID env vars, will merge into master locally and push (to avoid torturing CI)

@yarikoptic yarikoptic merged commit 3ea9e38 into datalad:master Nov 25, 2020
@vsoch
Copy link
Collaborator Author

vsoch commented Nov 25, 2020

Thanks @yarikoptic ! If you want a comparison workflow, we've been running tributors at USRSE for a few months now and it's working nicely so far https://github.com/USRSE/usrse.github.io/blob/master/.github/workflows/update-contributors.yml.

@vsoch vsoch deleted the add/tributors branch November 25, 2020 23:53
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.

None yet

2 participants