[Webkit-unassigned] [Bug 36394] Include git commits in the diff for webkit-patch upload/land.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=36394





--- Comment #62 from Ojan Vafai <ojan at chromium.org>  2010-05-25 14:16:40 PST ---
Sorry it took so long, but I'm just getting back to this.

(In reply to comment #61)
> It is probably better to throttle the rietveld logging from the calling code rather than by temporarily changing it from within.  Otherwise, it doesn't seem like there is a way for the caller to, say, temporarily re-enable this logging.

I agree, but I'm actually going to remove the logging filtering from this entirely. It's no longer something that will run on upload. It will only run on the upload bot or when explicitly run via webkit-patch post-attachment-to-rietveld. So, I think we'd actually prefer to do all the logging.

> > -        # FIXME: Add unit tests.
> > -        if git_commit and '..' in git_commit:
> > -            # FIXME: If the range is a "...", the code should find the common
> > -            #        ancestor and start there.  See git diff --help for how
> > -            #        "..." usually works.
> > -            self._parse_error('invalid --git-commit option: option does '
> > -                              'not support ranges "..": %s' % git_commit)
> 
> Above you deleted the FIXME to add more unit tests, but below you removed some unit tests without adding any new ones.  It seems like you should at least preserve the FIXME above to add more unit tests.

> Would it be possible for you to create optparser unit tests for the new options you are adding (and to update the existing tests to take into account the new options)?  Up to this point, we have maintained pretty good code coverage of the option parsing code.  Thanks.

I'll add tests for passing commit ranges to git-commit. The other options are getting deleted in bug 39624.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.