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

chore: automatic update contributors list #4029

Merged
merged 8 commits into from
Feb 8, 2022

Conversation

lukasholzer
Copy link
Collaborator

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes https://github.com/netlify/pod-workflow/issues/355

To keep our package.json contributors field up to date to value the effort of each individual person who is contributing to the CLI I'm adding a github actions that runs on every PR to update the contributors field.


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)
image

@lukasholzer lukasholzer added the type: chore work needed to keep the product and development running smoothly label Jan 13, 2022
@github-actions
Copy link

github-actions bot commented Jan 13, 2022

📊 Benchmark results

Comparing with 9975b53

Package size: 377 MB

⬆️ 0.00% increase vs. 9975b53

^                                                                                                  377 MB 
│  363 MB  363 MB  363 MB  362 MB  362 MB  362 MB  363 MB  363 MB  363 MB  362 MB  363 MB  363 MB   ┌──┐  
│ ──┌──┐────┌──┐────┌──┐────┌──┐────┌──┐────┌──┐────┌──┐────┌──┐────┌──┐────┌──┐────┌──┐────┌──┐────|▒▒|──
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

@lukasholzer lukasholzer marked this pull request as draft January 13, 2022 14:12
@lukasholzer lukasholzer force-pushed the chore/auto-update-contributors-list branch 7 times, most recently from d5fdaa4 to 890ff6f Compare January 14, 2022 12:13
@lukasholzer lukasholzer marked this pull request as ready for review January 14, 2022 12:18
@lukasholzer
Copy link
Collaborator Author

lukasholzer commented Jan 14, 2022

const packageJsonContributors = contributors.map((user) => {
const web = (user.twitter_username && `https://twitter.com/${user.twitter_username}`) || user.blog
let fullName = user.name || user.login
let email = mailList.get(user.login) || mailList.get(user.name) || user.email
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe someone has a better idea of getting the email address sadly there is nothing matching from the github api so I have to manual match them with the users

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we only show the person's URL, without the email address? This is what all-contributors does for example. This might be better from a privacy standpoint, and might help you solve that implementation problem.

Note: the all-contributors codebase might be useful to browse too to compare how they are getting the user information.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ahh thx :) I was just trying to get the information that was already provided in the package.json

check-latest: true
- name: Install dependencies
run: npm ci --no-audit
- name: Generate Contributors
Copy link
Contributor

Choose a reason for hiding this comment

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

We might be able to use https://github.com/actions/github-script#run-a-separate-file-with-an-async-function for this.

It will save us from installing the octokit dependencies directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to wait for actions/github-script#168 or use commonJS syntax

@erezrokah erezrokah force-pushed the chore/auto-update-contributors-list branch 2 times, most recently from 1ef1794 to afa187f Compare January 25, 2022 12:49
const PagedOctokit = Octokit.plugin(paginateRest)
const octokit = new PagedOctokit({ auth: GITHUB_TOKEN })

const contributorList = await octokit.paginate('GET /repos/{owner}/{repo}/contributors', {
Copy link
Contributor

@erezrokah erezrokah Jan 25, 2022

Choose a reason for hiding this comment

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

This will give us the existing contributors list, so I'm not sure if it includes first time contributors.
That is, if a first time user opens a PR, their name won't be added in that PR, but in the next one (I think), which can be confusing.

We probably want to look at the git log for the current commit, and use that as a reference

Copy link
Contributor

@erezrokah erezrokah Jan 26, 2022

Choose a reason for hiding this comment

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

Maybe a better way will be to take the PR author from the GitHub action event and use that one. It can save use the trouble of parsing git log and linking it to GitHub profiles

@lukasholzer
Copy link
Collaborator Author

@erezrokah I though moving this maybe in a seperate own github action that we can share across the organization? any thoughts on this? Will post it in our (internal) slack

@erezrokah
Copy link
Contributor

@erezrokah I though moving this maybe in a seperate own github action that we can share across the organization? any thoughts on this? Will post it in our (internal) slack

Yes please! This is a great idea

@erezrokah erezrokah force-pushed the chore/auto-update-contributors-list branch from 48226b6 to a00779f Compare February 8, 2022 10:01
erezrokah
erezrokah previously approved these changes Feb 8, 2022
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

@erezrokah erezrokah added the automerge Add to Kodiak auto merge queue label Feb 8, 2022
@kodiakhq kodiakhq bot merged commit fce24e7 into main Feb 8, 2022
@kodiakhq kodiakhq bot deleted the chore/auto-update-contributors-list branch February 8, 2022 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Add to Kodiak auto merge queue type: chore work needed to keep the product and development running smoothly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants