[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 #64 from Ojan Vafai <ojan at chromium.org>  2010-05-25 15:51:23 PST ---
> > > -        # 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.

Actually, I went to do this and realized that I don't know what this is talking about testing. This patch made commit ranges work, so the second FIXME above is addressed. I could add a test that optparser doesn't error our when you pass a commit range, but that seems sort of silly.

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