Skip to content

Fix scc check-prs by using find_branching_point()#158

Merged
sbesson merged 3 commits intoome:masterfrom
sbesson:checkprs_fix
Apr 4, 2014
Merged

Fix scc check-prs by using find_branching_point()#158
sbesson merged 3 commits intoome:masterfrom
sbesson:checkprs_fix

Conversation

@sbesson
Copy link
Copy Markdown
Member

@sbesson sbesson commented Apr 3, 2014

This commit addresses a tricky issue where a branch originated from an
ancestor commit is merged simultaneously in the source and target branches.
In that case, git merge-base will return this ancestor commit instead of
the true branching point. To fix this, CheckPRs now uses find_branching_point
already used in Rebase().


This PR should fix http://ci.openmicroscopy.org/view/Mgmt/job/SCC-check-prs/.
To test this PR in context, run python scc/main.py check-prs dev_5_0 develop from openmicroscopy.git and check the output is valid.

This commit addresses a tricky issue where a branch originated from an
ancestor commit is merged simultaneously in the source and target branches.
In that case, `git merge-base` will return this ancestor commit instead of
the true branching point. To fix this, CheckPRs now uses find_branching_point
already used in Rebase().
@joshmoore
Copy link
Copy Markdown
Member

Before testing, etc. a quick thought: does anything else use "merge-base"?

@sbesson
Copy link
Copy Markdown
Member Author

sbesson commented Apr 3, 2014

Yes a quick grep shows already-merged uses it. Want me to fix that?

@joshmoore
Copy link
Copy Markdown
Member

SVP.

@sbesson
Copy link
Copy Markdown
Member Author

sbesson commented Apr 3, 2014

Actually, going through the command again, we may want to use exactly git merge-base in this particular case.

@joshmoore
Copy link
Copy Markdown
Member

Hmmm....that seems slightly surprising, but I guess it's possible.

sbesson added 2 commits April 3, 2014 20:05
This should allow get_rev_list() to work for submodules
@mtbc
Copy link
Copy Markdown
Member

mtbc commented Apr 4, 2014

After latest commit, seems to be working great, @joshmoore willing then good to merge.

@joshmoore
Copy link
Copy Markdown
Member

Haven't had time to test. Don't let me hold up the release. Certainly having check-prs back online is more important than already-merged.

@sbesson
Copy link
Copy Markdown
Member Author

sbesson commented Apr 4, 2014

@joshmoore: since last commit was pushed today, I will wait for http://ci.openmicroscopy.org/view/Mgmt/job/SCC-merge/ output first.
If this is okay with you and @mtbc, I will tag and release scc on Monday (with 5.0.1) and have the check-prs jobs running again for Tuesday.

@joshmoore
Copy link
Copy Markdown
Member

Fine by me.

@sbesson
Copy link
Copy Markdown
Member Author

sbesson commented Apr 4, 2014

sbesson added a commit that referenced this pull request Apr 4, 2014
Fix scc check-prs by using find_branching_point()
@sbesson sbesson merged commit 79cc151 into ome:master Apr 4, 2014
@sbesson sbesson deleted the checkprs_fix branch April 4, 2014 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants