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

Use `git diff` instead of `git log` for --changedSince #10155

Merged
merged 11 commits into from Jul 3, 2020

Conversation

@finn-orsini
Copy link
Contributor

finn-orsini commented Jun 10, 2020

Summary

Addresses issue described in #9602, where files with no difference are marked as changed by using git log instead of git diff for the --changedSince flag option.

Test plan

  • All existing git --changedSince specs pass
  • Added spec for the case described in #9602, where the committed changes have no diff, verifies no file names returned from getChangedFilesForRoots.

Note: Seeing failures on local master branch with message abort: empty revision range for jestChangedFiles: gets changed files for hg and onlyChanged: gets changed files for hg specs after running brew install hg successfully.

Seraphina Orsini
@SimenB
SimenB approved these changes Jun 23, 2020
Copy link
Collaborator

SimenB left a comment

I think this makes sense (as discussed in the issue). Thanks for sending a PR!

/cc @jeysal @thymikee @cpojer any final thoughts?


Regarding the Mercurial tests, I haven't had them pass for at least a year (and they're skipped on CI) so feel free to ignore them

CHANGELOG.md Outdated Show resolved Hide resolved
finn-orsini and others added 2 commits Jun 24, 2020
Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>
@matthew-hallsworth
Copy link

matthew-hallsworth commented Jul 1, 2020

Can we get an update as to when this might be merged and released? I have a few things in our company test pipeline that rely upon this change :)

Thanks!

@alsuren
alsuren approved these changes Jul 2, 2020
Copy link
Contributor

alsuren left a comment

This looks like it will do the right thing in all of the cases that I care about (including when branches diverge). Thanks for taking this on.

@SimenB
Copy link
Collaborator

SimenB commented Jul 3, 2020

Good enough for me. Thanks @finn-orsini!

SimenB added 2 commits Jul 3, 2020
@SimenB SimenB merged commit e936da2 into facebook:master Jul 3, 2020
15 of 22 checks passed
15 of 22 checks passed
cleanup-runs
Details
Running TypeScript compiler & ESLint
Details
Node v10.x on ubuntu-latest
Details
Node v10.x on macOS-latest
Details
Node v10.x on windows-latest
Details
Node v12.x on ubuntu-latest
Details
Node v12.x on macOS-latest
Details
Node v12.x on windows-latest
Details
Node v13.x on ubuntu-latest
Details
Node v13.x on macOS-latest
Details
Node v13.x on windows-latest
Details
Node v14.x on ubuntu-latest
Details
Node v14.x on macOS-latest
Details
Node v14.x on windows-latest
Details
ci/circleci: test-jest-circus CircleCI is running your tests
Details
ci/circleci: test-node-10 CircleCI is running your tests
Details
ci/circleci: test-node-12 CircleCI is running your tests
Details
ci/circleci: test-node-13 CircleCI is running your tests
Details
ci/circleci: test-node-14 CircleCI is running your tests
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
facebook.jest queued
Details
ci/circleci: test-or-deploy-website Your tests passed on CircleCI!
Details
@finn-orsini finn-orsini deleted the finn-orsini:sorsini_9602_changed_since branch Jul 3, 2020
@pelotom
Copy link

pelotom commented Jul 8, 2020

Any chance this could get released soon? This is a major blocker for us since we can't practically run all of our tests in CI on every PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.