Skip to content
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 format-diff.sh detecting changes vs. upstream #5831

Closed
wants to merge 2 commits into from

Conversation

pdillinger
Copy link
Contributor

@pdillinger pdillinger commented Sep 19, 2019

Summary: format-diff.sh, a.k.a. 'make format', would use 'master'
to decide which commits are probably unpublished. Much better to use
facebook remote master since local master may not be caught up and may
have its own unpublished commits. Script now tries to compare against
facebook remote master branch (branch pointer is updated with any fetch
or pull), because those differences are what would be considered the
differences for a pull request.

Also, script would compare against parent of merge-base with that
reference point, which is just wrong since that includes the last
published commit.

In case of problems, you can now customize the reference point, by
setting the FORMAT_UPSTREAM variable.

Test Plan: manual

build_tools/format-diff.sh Outdated Show resolved Hide resolved
Summary: format-diff.sh, a.k.a. 'make format', would use 'master'
to decide which commits are probably unpublished. Much better to use
facebook remote master since local master may not be caught up and may
have its own unpublished commits. Script now tries to compare against
facebook remote master branch (branch pointer is updated with any fetch
or pull), because those differences are what would be considered the
differences for a pull request.

Also, script would compare against *parent* of merge-base with that
reference point, which is just wrong since that includes the last
published commit.

In case of problems, you can now customize the reference point, by
setting the FORMAT_UPSTREAM variable.

Test Plan: manual
Copy link
Contributor

@siying siying left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see how it works.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pdillinger is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@pdillinger merged this pull request in 42f898b.

merryChris pushed a commit to merryChris/rocksdb that referenced this pull request Nov 18, 2019
Summary:
format-diff.sh, a.k.a. 'make format', would use 'master'
to decide which commits are probably unpublished. Much better to use
facebook remote master since local master may not be caught up and may
have its own unpublished commits. Script now tries to compare against
facebook remote master branch (branch pointer is updated with any fetch
or pull), because those differences are what would be considered the
differences for a pull request.

Also, script would compare against *parent* of merge-base with that
reference point, which is just wrong since that includes the last
published commit.

In case of problems, you can now customize the reference point, by
setting the FORMAT_UPSTREAM variable.
Pull Request resolved: facebook#5831

Test Plan: manual

Differential Revision: D17528462

Pulled By: pdillinger

fbshipit-source-id: 50fdb8795d683bf3c14d449669c1a5299e0dfa8b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants