-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
gh-428: Add GitHub issue to description of PR #430
gh-428: Add GitHub issue to description of PR #430
Conversation
@ryanlong1004 Please can you resolve the conflicts? |
Codecov Report
@@ Coverage Diff @@
## main #430 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 18 18
Lines 1720 1760 +40
Branches 208 213 +5
=========================================
+ Hits 1720 1760 +40
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
BODY = f"""\ | ||
{{body}} | ||
gh-{{issue_number}} | ||
{CLOSING_TAG} |
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.
Why is there a closing tag but no opening tag?
I'm not sure what the tag is for, but my best guess is that it's so the bot can reliably recognize that it has already added a link. So maybe we should also add an open tag and use that to check whether to add the issue number.
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.
Taken from link below. There was no opening tag there.
https://github.com/python/bedevere/blob/3d532cd90271a506ad222af42dbaacb8139c1138/bedevere/bpo.py
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.
@JelleZijlstra, can this conversation be resolved?
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.
The old code did have an opening tag:
Line 15 in 3d532cd
<!-- {TAG_NAME}: bpo-{{issue_number}} --> |
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.
Though it seems we're not using the tag now. If so, we should remove the closing tag too.
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 think the tag was added there to make it easier to parse it, e.g. if one day we migrate again.
Thanks for your work @ryanlong1004! Would you be able to increase the coverage to 100%? |
@Mariatta, you requested changes. Are you happy with the PR as it stands? Please mark it approved, if so. |
Closes #428