multiroot jest-change-files #3969
Conversation
| return new Promise((resolve, reject) => { | ||
| let args = ['status', '-amnu']; | ||
| if (options && options.withAncestor) { | ||
| args.push('--rev', 'ancestor(.^)'); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Maybe ask internally, but I think that's about what it does. If it's only used internally, that's probably why I added this here and we can kill it :D
aaronabramov
force-pushed
the
jest-changed-files
branch
from
91d0724
to
dd9c085
Compare
integration_tests/utils.js
Outdated
| @@ -29,6 +29,9 @@ const run = (cmd, cwd) => { | |||
| throw new Error(message); | |||
| } | |||
|
|
|||
| result.stdout = String(result.stdout); | |||
There was a problem hiding this comment.
I'd feel better about this if you called toString() explicitly.
integration_tests/utils.js
Outdated
| @@ -57,7 +60,8 @@ const makeTemplate = string => { | |||
| }; | |||
| }; | |||
|
|
|||
| const cleanup = (directory: string) => rimraf.sync(directory); | |||
| const cleanup = (directory: string) => | |||
| fs.existsSync(directory) && rimraf.sync(directory); | |||
| stdout = stdout.trim(); | ||
| if (stdout === '') { | ||
| resolve([]); | ||
| const adapter: SCMAdapter = { |
There was a problem hiding this comment.
Do you need this explicit typing here?
There was a problem hiding this comment.
yeah. to make sure both git and hg have the same API and don't deviate
|
This looks fine overall, but I'm not entirely sure about the plan here. This doesn't really change anything, does it? Are you planning to take the main call out of |
| options: Options, | ||
| ): Promise<Array<Path>> => { | ||
| return new Promise((resolve, reject) => { | ||
| let args = ['status', '-amnu']; |
There was a problem hiding this comment.
also i think i found a bug here. the original arguments were -amn and that does not return untracked files. ./git.js does return untracked files
aaronabramov
force-pushed
the
jest-changed-files
branch
6 times, most recently
from
3e921ef
to
08cb3c5
Compare
aaronabramov
force-pushed
the
jest-changed-files
branch
from
08cb3c5
to
0ef151e
Compare
Codecov Report
@@ Coverage Diff @@
## master #3969 +/- ##
=========================================
- Coverage 59.92% 59.8% -0.13%
=========================================
Files 196 196
Lines 6785 6769 -16
Branches 6 6
=========================================
- Hits 4066 4048 -18
- Misses 2716 2718 +2
Partials 3 3
Continue to review full report at Codecov.
|
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
first step towards optimizing performance of this module.
the problem we hit at fb is that we have too many roots specified for every project, and we had to get changed files for each root.
Now all roots are first resolved to a SCM root and deduped, so we'll be querying SCM only once per context.
includes a shitload of tests and types.
todo (followup PRs):
www,mobileare in the same repo and will give different results, but with the current implementation it'll do one request for every context)