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

feat(schematics): affected support for uncommitted changes #351

Closed
wants to merge 1 commit into from

Conversation

markphip
Copy link
Contributor

Extended the syntax for affected so that it can get the
list of files from either locally staged changes or all
modified but unstaged files. Syntax is:

nx affected apps staged
nx affected apps unstaged

This could enable developer to more easily run a subset
of tests locally before committing.

Related to #201

Extended the syntax for affected so that it can get the
list of files from either locally staged changes or all
modified but unstaged files.  Syntax is:

nx affected apps staged
nx affected apps unstaged

This could enable developer to more easily run a subset
of tests locally before committing.

Related to nrwl#201
@markphip
Copy link
Contributor Author

I do not know how to actually test this but it seems simple and might be useful to someone and that it would work. You might want to handle the syntax differently than I did it.

@@ -51,6 +56,22 @@ function parseDashDashFiles(dashDashFiles: string): string[] {
return f.split(',').map(f => f.trim());
}

function getFilesFromLocalChanges(staged: boolean): string[] {
if (staged) {
return execSync(`git diff --name-only --cached`)
Copy link
Member

Choose a reason for hiding this comment

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

Some code duplication. Maybe create a getFilesNamesFromGitOutput helper?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Better yet, simple-git handles this more nicely. It gives nice JSON format results from git commands. https://github.com/steveukx/git-js. If we want to bring in that dependency

@vsavkin
Copy link
Member

vsavkin commented Mar 25, 2018

I like the changes. I think it's a useful addition.

How important do you think to differentiate between the staged and the unstaged files?

Maybe something like this will suffice?

npm run affected:build -- --uncommitted

A lot of folks don't know the difference between the two and think about it as "uncommitted".
We can also have the following three commands:

npm run affected:build -- --uncommitted
npm run affected:build -- --staged
npm run affected:build -- --unstaged

What do you think?

@markphip
Copy link
Contributor Author

I am sure there are some people that use staging and might want the --staged option, but I agree that most people probably just want --uncommitted. One reason I did not pursue that option is I am unclear what the reliably since git diff command is to obtain that list. Would you just run both commands and use the union of returned paths, or are you aware of a single command that can give you this list?

It sounds like you are also proposing a better syntax too. I would tend to agree with that but it is probably over my head to see this to completion. So hopefully someone else can do that.

@vsavkin
Copy link
Member

vsavkin commented Mar 26, 2018

I was thinking about merging the outputs of the two command.

I'll finish this work. Thank you!

@markphip
Copy link
Contributor Author

Thanks for taking it to the finish line.

FWIW, I did a little research yesterday that I just tried and it seems to work. This command will give you the output of uncommitted:

$ git diff --name-only HEAD .

It will not return new files that have not been git added, but I do not think there is any git diff command that will do that. If we wanted to include those files, I think we would need new code that gets the paths from this command:

$ git status -s

This gives similar output to git diff, except it adds a column in front of the file name that has status flags for modifed, deleted, unversioned etc..

@vsavkin
Copy link
Member

vsavkin commented Mar 27, 2018

Merged here 113b51b

Thank you!

@vsavkin vsavkin closed this Mar 27, 2018
@markphip
Copy link
Contributor Author

Thanks. I noticed you did not update the help output that I had added. Do you want to fix that or have me send you a PR? It still has the staged unstaged terms I originally introduced

@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants