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
Add a git webkit file-bugs that creates bugzilla issues from existing commits. #5513
base: main
Are you sure you want to change the base?
Conversation
EWS run on previous version of this PR (hash cb1bb00) |
cb1bb00
to
16a176d
Compare
EWS run on previous version of this PR (hash 16a176d) |
# logic for choosing the branch point. Given that not having an issue is the whole | ||
# reason we're running this code, it might be a sign that this logic isn't going to | ||
# be correct for what we want. | ||
args.issue = None |
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.
Maybe we should just make PullRequest.pull_request_branch_point
work without the issue argument than force the caller to pass in an empty one. It should work with issue = None
, if I recall what we're doing in that function.
|
||
# FIXME: This just uses the exact same logic as pull-request for selecting | ||
# a branch. It would be nice if we had a second option to specify an arbitrary | ||
# commit (range?). |
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.
That would be repository.commits()
, but I suppose really, it's whatever your base is, right? You're just (I think correctly) using the same logic to find the base commit. If the user specified a base commit, you'd use that over the branch_point
, I expect.
aliases = ['bugs'] | ||
help = 'File bugzilla issues for commits without one specified' | ||
|
||
BUGZILLA_PLACEHOLDER = 'Need the bug URL (OOPS!).' |
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.
This is more generic, it's actually just "BUG_PLACEHOLDER" (we may move to GitHub issues at some point)
) | ||
|
||
# Probably want some more args to configure the behaviour around | ||
# auto-editing commit messages etc |
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.
Also want to let the user specify their own branch_point
, yes?
|
||
# Check if there's something on the 3rd line (for a rdar:) | ||
# If so, then start our search for the description one line | ||
# later. |
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 commit object centralizes this logic in the issues
property. But more to the point of this function, I've found it's most reliable to use empty lines, commit messages are usually in the format:
<title>
<links>
// Empty line
Reviewed by...
// Empty line
<description>
// Empty line
<files changed>
# Search for a description from the 7th line until we see a '*' for file changes. | ||
# (should be title,bugzilla,rdar?,blank,reviewer,blank on first 5/6 lines). | ||
# FIXME: This is very hardcoded to the exact template, could probably be a bit more | ||
# adaptive. |
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 looking for '*' is a good idea, that's a pretty strong requirement in our commit message templates. Especially if you did line.lstrip()
to strip any leading whitespace from lines.
issue = None | ||
radar = None | ||
needs_radar = False | ||
if cls.BUGZILLA_PLACEHOLDER not in lines[1]: |
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 for this bit, you want the project's default issue tracker (can get that from Tracker.instance()
)
from webkitbugspy import Tracker, radar | ||
from webkitcorepy import arguments, run, Terminal | ||
from webkitscmpy import local, log, remote | ||
from webkitpy.common.system.user import User |
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.
We can't use webkitpy
in webkitscmpy
because webkitpy
isn't a pip package (and webkitcorepy
/webkitscmpy
/webkitbugspy
are basically an effort to fix that deficiency of webkitpy
. webkitcorepy.Terminal
has some of the same functionality.
# changes that our amend would accidentally bring in. Is this a sufficient check? | ||
# Detached HEAD state? Error checking on the amend git command? | ||
# Should we pre-warn at the start of the command if we won't be able to write | ||
# the results? |
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.
This comment makes me wonder if we're better off just operating on a single commit....otherwise, I think we need to use git filter-branch
...it's possible, git-webkit canonicalize
uses git filter-branch
extensively, but it's a really hard command to use.
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 it's worth having the ability to file bugs for a range of commits, even if you have to do the git rebase -I
manually afterwards to insert them into the commit messages. That's still a lot less manual work than filing the bugs by hand, or moving commits around so that each of them ends up without descendants.
β¦ commits. https://bugs.webkit.org/show_bug.cgi?id=246381 <rdar://problem/101181555> Reviewed by NOBODY (OOPS!). * Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/__init__.py: * Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/file_bugs.py: Added. (FileBugs): (FileBugs.parser): (FileBugs.parse_description): (FileBugs.create_bug): (FileBugs.create_bugs): (FileBugs.main):
16a176d
to
043ac3c
Compare
EWS run on current version of this PR (hash 043ac3c) |
043ac3c
043ac3c