-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix frozen tools check never failing #14705
Conversation
@LDong-Arm, thank you for your changes. |
We only compare files in current PR with the base branch. There's no need to fetch the full git history or branches that are not involved in the comparison. This is to save time in Travis runs.
4ce15cf
to
3b515b3
Compare
Having recently moved the frozen tools check to a standalone stage at the end, we need to fetch the base branch so make comparison work. Travis environments are not shared across different stages. Without the fetch, the check always passes as a PR is compared with itself.
The filter `--diff-filter=d' causes deleted files to be ignored from the frozen tools check. But there is no reason for this, as deleting a file can break compatibility with the legacy tools.
3b515b3
to
a1d1d55
Compare
@LDong-Arm, thank you for your changes. |
a1d1d55
to
e31f2bb
Compare
Now ready for review. I've removed the experimental commit. Frozen tools check failed as expected when the experimental commit was there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
CI started |
Summary of changes
Having recently moved the frozen tools check to a standalone Travis stage at the end, we need to fetch the base branch so make comparison work. Travis environments are not shared across different stages. Without the fetch, the check always passes as a PR is compared with itself.
Also change
git fetch
to be shallow, as the git history is irrelevant togit diff
which only compares files between two revisions.And deleted files are no longer ignored from the frozen tools check, because deleting a file can also break compatibility with the legacy tools.
Impact of changes
Migration actions required
Documentation
None.
Pull request type
Test results
Reviewers