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

Possibility to see affected apps without commiting the code or passing list of changed files #201

Closed
skryvets opened this issue Jan 12, 2018 · 4 comments

Comments

@skryvets
Copy link

Hello everyone.

I posted this question on StackOverflow but didn't get a lot of impressions, since nx is a very specific to certain type of projects. Currently, I do some proof of concept and want to research all options which are available in nx and hope to get some insights here.

Prerequisites:

One of the most powerful features of NX is to able to see which Angular applications are affected by the recent change.

However, in order to make it work documentation tells that you need:

  1. Provide SHA's of two commits: npm run apps:affected -- SHA1 SHA2 OR
  2. Specify each file that has been changed and pass them as parameters: npm run apps:affected -- --files="libs/mylib/index.ts,libs/mylib2/index.ts"

Obviously, the first option doesn't feet as I don't want to commit my changes and see what's broken. The second option doesn't work as well because it really requires a lot of effort to pass each changed file as a parameter.

Question:

Is there a way that I can change a file in a lib and see what apps are affected without passing the commit SHA or manually specifying each change? (probably just by analyzing git diff).

Thank you!

@Yonet
Copy link
Contributor

Yonet commented Jan 12, 2018

@sergeome you can run your tests to see all the apps that are effected. The options you have mentioned are for the case you don't want to run all of your tests.

@Yonet Yonet closed this as completed Jan 12, 2018
@skryvets
Copy link
Author

@Yonet, thank you for your answer.

Yes, you're totally right. The point is I have a lot of tests and it takes some time to go through all. Before committing, I wanted to run tests only against apps that are affected by my change without committing or comparing a branch. In order to do that, I wanted to know whether nx can help with that by determining which apps are affected (to run specific tests later on).

Is that something that can be possibly added in the future? This might be not a relevant case, but for right now I don't see why.

Thank you!

@markphip
Copy link
Contributor

I've been looking at the affected feature to streamline our CI. That got me to searching your issues which led me to this one. It is not related to my current needs but I was intrigued by it. I think the example given was not great because obviously if you only changed a single file in a lib then the current CLI of just passing that filename is probably the easiest. Clearly the issue is when you have a lot of local changes.

This seems like it would be easy to implement support for. If you pass a SHA1 and SHA2 it just uses those with git diff to get the list of files. So why not just come up with a syntax that does the same thing based on the current uncommitted changes? I am not sure the best CLI syntax for this so I just hacked up a quick example I will attach the diff for. In this example, I am just overloading the SHA1 SHA2 syntax to support some "magic" strings. So you could do either something like this:

nx affected apps local staged

And that would use your current staged changes to determine the files. Or this to base it on unstaged changes:

nx affected apps local unstaged

I am not sure how to test this out, and I doubt this is the CLI syntax you would want to use if you like this idea, so rather than send a PR I figured I would just attach a diff here as an example of what I mean. I can see where this would be a useful feature for a dev to run tests based on the code they had changed before committing.

diff --git a/packages/schematics/src/command-line/shared.ts b/packages/schematics/src/command-line/shared.ts
index afd99e6..e6cc69f 100644
--- a/packages/schematics/src/command-line/shared.ts
+++ b/packages/schematics/src/command-line/shared.ts
@@ -52,6 +52,21 @@ function parseDashDashFiles(dashDashFiles: string): string[] {
 }
 
 function getFilesFromShash(sha1: string, sha2: string): string[] {
+  if (sha1.startsWith('local')) {
+     if (sha2.startsWith('staged')) {
+        return execSync(`git diff --name-only --cached`)
+        .toString('utf-8')
+        .split('\n')
+        .map(a => a.trim())
+        .filter(a => a.length > 0);
+     } else {
+        return execSync(`git diff --name-only`)
+        .toString('utf-8')
+        .split('\n')
+        .map(a => a.trim())
+        .filter(a => a.length > 0);
+     }
+  }
   return execSync(`git diff --name-only ${sha1} ${sha2}`)
     .toString('utf-8')
     .split('\n')

markphip added a commit to markphip/nx that referenced this issue Mar 21, 2018
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
vsavkin pushed a commit that referenced this issue Mar 26, 2018
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
vsavkin pushed a commit that referenced this issue Mar 27, 2018
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
vsavkin pushed a commit that referenced this issue Mar 27, 2018
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
@github-actions
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

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

No branches or pull requests

3 participants