Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Make git diff range more robust in format.sh#5813

Merged
liyuqian merged 3 commits intoflutter:masterfrom
liyuqian:format
Jul 20, 2018
Merged

Make git diff range more robust in format.sh#5813
liyuqian merged 3 commits intoflutter:masterfrom
liyuqian:format

Conversation

@liyuqian
Copy link
Contributor

The new approach follows flutter/flutter#19412 which is suggested by Cirrus in cirruslabs/cirrus-ci-docs#17

The new approach follows flutter/flutter#19412
which is suggested by Cirrus in cirruslabs/cirrus-ci-docs#17
@liyuqian
Copy link
Contributor Author

@Hixie : this should fix your Travis problem in #5810 and I think that this modification is useful no matter whether we use Travis or Cirrus.

@liyuqian
Copy link
Contributor Author

@gspencergoog : I want to drop the --fork-point as it seems to be unnecessary, and sometimes causing some troubles. What's your opinion?

@gspencergoog
Copy link
Contributor

In what cases does it cause problems?

@liyuqian
Copy link
Contributor Author

@gspencergoog : when my local branch HEAD is based on a commit earlier than FETCH_HEAD, git merge-base --forkpoint FETCH_HEAD HEAD returns empty while git merge-base FETCH_HEAD HEAD returns the correct common ancestor.

@gspencergoog
Copy link
Contributor

gspencergoog commented Jul 20, 2018

Oh, I see. Maybe we should try with --fork-point first, and fall back to plain merge-base if it fails.

Only because --fork-point seems to be more conservative about getting the "right" commit.

Or, we could try just leaving it off and see if we get any odd failures...

@liyuqian
Copy link
Contributor Author

Done. We now fall back to git merge-base if git merge-base --fork-point fails.

@gspencergoog
Copy link
Contributor

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

@liyuqian liyuqian merged commit 2a77a41 into flutter:master Jul 20, 2018
@liyuqian liyuqian deleted the format branch September 12, 2018 22:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants