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

Disable alternate diff drivers in setup script #10378

Merged
merged 1 commit into from Oct 6, 2018

Conversation

Projects
None yet
2 participants
@chrisvanpatten
Contributor

chrisvanpatten commented Oct 6, 2018

If you have an alternate diff driver set up (in my case it's a small shell script wrapping vimdiff), the setup script can hang when running git diff --exit-code package.json to check if package.json has changed. (Basically, Vim can't load the UI so you can close it and return the exit code, and throws up an error.)

This change adds the --no-ext-diff flag to those checks, which disables any external/custom diff drivers and uses git's built-in driver instead.

It's a small corner case but enforcing the use of git's internal diff driver should help guarantee slightly more consistency in general.

Testing

Run git diff --exit-code package.json and git diff --no-ext-diff --exit-code package.json and confirm they return the same exit code.

Or run the whole setup script (or just ./bin/install-node-nvm.sh) if you want πŸ€·β€β™‚οΈ

@chrisvanpatten chrisvanpatten requested a review from WordPress/gutenberg-core Oct 6, 2018

@tofumatt

Huh, today I learned!

Makes sense to me; return codes are the same to me. πŸ‘

@chrisvanpatten chrisvanpatten merged commit 1e72714 into WordPress:master Oct 6, 2018

2 checks passed

codecov/project 49.31% remains the same compared to 86dad5f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tofumatt

This comment has been minimized.

Show comment
Hide comment
@tofumatt

tofumatt Oct 6, 2018

Member

Ah, quick note though: could you look through our scripts and ensure we are using --no-ext-diff everywhere? I think we use it elsewhere to do some checks; it's probably worth doing it this way everywhere.

Member

tofumatt commented Oct 6, 2018

Ah, quick note though: could you look through our scripts and ensure we are using --no-ext-diff everywhere? I think we use it elsewhere to do some checks; it's probably worth doing it this way everywhere.

@chrisvanpatten chrisvanpatten deleted the TomodomoCo:fix/avoid-alternate-diff-tools branch Oct 6, 2018

@tofumatt

This comment has been minimized.

Show comment
Hide comment
@tofumatt

tofumatt Oct 6, 2018

Member

Haha, you are TOO FAST. πŸ˜†

Member

tofumatt commented Oct 6, 2018

Haha, you are TOO FAST. πŸ˜†

@chrisvanpatten

This comment has been minimized.

Show comment
Hide comment
@chrisvanpatten

chrisvanpatten Oct 6, 2018

Contributor

@tofumatt I'm on the ball this morning πŸ˜‚πŸƒβ€β™‚οΈ

Contributor

chrisvanpatten commented Oct 6, 2018

@tofumatt I'm on the ball this morning πŸ˜‚πŸƒβ€β™‚οΈ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment