8000 Fix wrong PR URL in logs and log closed PR labels by yajo · Pull Request #33 · acsone/git-aggregator · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix wrong PR URL in logs and log closed PR labels #33

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

Merged
merged 2 commits into from
Aug 14, 2019

Conversation

yajo
Copy link
Contributor
@yajo yajo commented Aug 12, 2019

Before this patch, an URL with /pulls/ appeared in the logs, when the correct one is with /pull/ instead.

It turns out the GitHub API returns the browsable URL in the response, so I use that one instead.

Also, bots tend to mark PRs as closed instead of merged, but add some label that indicates such state. Let's log labels to have that information at hand.

Before this patch, an URL with `/pulls/` appeared in the logs, when the correct one is with `/pull/` instead.

It turns out the GitHub API returns the browsable URL in the response, so I use that one instead.
@yajo yajo changed the title Log closed PR labels. Fix wrong PR URL in logs and log closed PR labels Aug 12, 2019
Copy link
Member
@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

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

Tested. Thanks!

@sbidoul
Copy link
Member
sbidoul commented Aug 12, 2019

@yajo could you add a changelog entry in the readme?

Bots tend to mark PRs as closed instead of merged, but add some label that indicates such state. Let's log labels to have that information at hand.
@yajo yajo force-pushed the merged-prs-fix-imp branch from d5b017a to fbcbf99 Compare August 14, 2019 06:23
@yajo
Copy link
Contributor Author
yajo commented Aug 14, 2019

Done, thanks. I've put the date of today optimistically 😇.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.3%) to 81.593% when pulling fbcbf99 on Tecnativa:merged-prs-fix-imp into 8631b0e on acsone:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.3%) to 81.593% when pulling fbcbf99 on Tecnativa:merged-prs-fix-imp into 8631b0e on acsone:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 82.418% when pulling fbcbf99 on Tecnativa:merged-prs-fix-imp into 8631b0e on acsone:master.

@sbidoul sbidoul merged commit c227295 into acsone:master Aug 14, 2019
@sbidoul
Copy link
Member
sbidoul commented Aug 14, 2019

Ok, merged and tagged. It should land on pypi soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0