[Webkit-unassigned] [Bug 38852] webkit-patch land --squash commits too much if branch is not up to date

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Ojan Vafai <ojan at chromium.org>  2010-05-21 16:07:02 PST ---
(In reply to comment #8)
>  603                     raise ScriptError(message=self._get_squash_error_message(num_local_commits))
> 
> ScriptErorr has historically meant "there was an error running the script".  Used by run_command.  I think you want some other type of exception here.  Maybe not?  I know our infrstrcture is set up to handle ScriptERrors well, maybe tthat's what your'e going for here?

Yeah, using another error type gives an ugly stacktrace. Using ScriptError gives a nice, readable error message.

> I still think distingushing false from None doesn't help us here:

The point is that if you explicitly pass --squash or --no-squash  on the commandline it should override any other settings. So it shouldn't check the git config or anything. If you don't explicitly set it, then we need to do the other checks. 

It's gross, I know. But it will go away shortly when we replace --squash with --git-commit=* as per the webkit-patch discussion.

> Is this really the way you want to do this?
>      def _svn_branch_has_extra_commits(self):
>  612         # When head contains all the commits on the svn branch, show-branch --topics will have exactly 4 lines.
>  613         # One line for head, one for trunk, one for the "--" seprator and one for the branch point on trunk.
>  614         trunk_commits = run_command(['git', 'show-branch', '--topics', 'head', self.svn_branch_name()])
>  615         return len(re.findall('\n', trunk_commits)) != 4
> 
> Seems like a strange hack.  Maybe I'm mis-remembering evan's recomendation... if he had one?
> 
> This is better than what we have, but It feels like a hack.  Please consider the above comments, especially the one about the "4 line" hack.  Are we testing the 4-line hack?

Yeah, I was just being dumb here. Changing it to use rev-list as evan suggested.

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