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

Update the changelog script to include author in the oneliner #2323

Closed
tsandall opened this issue Apr 21, 2020 · 5 comments · Fixed by #2376
Closed

Update the changelog script to include author in the oneliner #2323

tsandall opened this issue Apr 21, 2020 · 5 comments · Fixed by #2376
Assignees
Labels

Comments

@tsandall
Copy link
Member

We like to recognize people for contributing to OPA by including a mention of them in the CHANGELOG. Today is this is somewhat ad-hoc and requires manual edits to the generated CHANGELOG updates. It would be better if the changelog script (build/changelog.py) was smart enough to figure out the GitHub name of the person.

@Syn3rman
Copy link
Contributor

Syn3rman commented May 6, 2020

Hey @tsandall,

I'd like to give this a shot, so can I try this if this is still open?

This would aim at adding the author to the changelog so that it looks something like <fix> <fixed by> <author> right?

Also, I tried running the script but I'm getting the error

Traceback (most recent call last):
  File "changelog.py", line 88, in <module>
    main()
  File "changelog.py", line 65, in main
    args = parse_args()
  File "changelog.py", line 57, in parse_args
    default=get_latest_tag(), help="start of changes")
  File "changelog.py", line 49, in get_latest_tag
    return run(cmd).split('-')[0]
TypeError: a bytes-like object is required, not 'str'

while running the script, the fix for which would be to use return subprocess.check_output(shlex.split(cmd), *args, **kwargs).decode('utf-8') instead, so does this need to be done or am I getting something wrong?

Update:
I changed the script so that the changelog messages are in the format:

- ast, topdown: Index comprehensions to avoid unnecessary work ([#2276](https://github.com/open-policy-agent/opa/issues/2276)) authored by Torin Sandall

And if we want the GitHub id of the user, one way would be to use the GitHub search API using:

res = requests.get(url, params=body)
user = res.json()["items"][0].get("login","")

@patrick-east
Copy link
Contributor

Thanks for looking into this one @Syn3rman !! Looks like you're already making some good progress on it 👍

I do think that we would probably want to try and lookup their GitHub id, ideally once we have the commit SHA ID we should be able to lookup the metadata about it via https://developer.github.com/v3/repos/commits/#get-a-single-commit the snippet using requests is probably pretty close to what we'd want to use.

@tsandall
Copy link
Member Author

tsandall commented May 6, 2020

Getting the login name from the API that @patrick-east showed would be ideal because they contributors will get tagged and anyone looking at the release notes will be able to easily navigate to their profile.

If the script could exclude OPA maintainers that would be best. I'm not sure the best way to achieve that. If it's possible to lookup the organizations that a GitHub user is apart of that would help. I think my own organization memberships are public.

@patrick-east
Copy link
Contributor

May not be too hard to just check against https://github.com/open-policy-agent/opa/blob/master/MAINTAINERS.md too

@Syn3rman
Copy link
Contributor

Syn3rman commented May 6, 2020

Thanks for the reply @tsandall @patrick-east!

I do think that we would probably want to try and lookup their GitHub id, ideally once we have the commit SHA ID we should be able to lookup the metadata about it via https://developer.github.com/v3/repos/commits/#get-a-single-commit the snippet using requests is probably pretty close to what we'd want to use.

That seemed to work for me till I ran into a 403 error, since it looks like you can only make 60 requests while you are not authenticated.

If the script could exclude OPA maintainers that would be best. I'm not sure the best way to achieve that. If it's possible to lookup the organizations that a GitHub user is apart of that would help. I think my own organization memberships are public.

I thought of first checking if the author is a maintainer or not by using the regex pattern [^\s]+@[^\s]+ to extract the email for all the emails from MAINTAINERS.md and making the request only if they aren't, which would also reduce the number of API calls required.

I have implemented this, so I'll open a pr so that you can have a look at it and suggest changes.

patrick-east pushed a commit that referenced this issue May 21, 2020
OPA uses the changelog file to recognize people who contribute to the
project by mentioning them in it. This commit introduces a way to fetch
the GitHub handle for the commit authors if they are not listed in the
MAINTAINERS.md file. It makes use of the GitHub API to do so, and thus
introduces an optional cli argument to pass the token to authenticate
requests, without which you might run into rate limit errors if you run
the script a couple of times.

Fixes #2323
Signed-off-by: Aditya <aditya10699@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants