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

npm version --no-git-tag-version --verbose should not log git error #263

Closed
wants to merge 1 commit into from

Conversation

woppa684
Copy link

@woppa684 woppa684 commented Oct 7, 2019

When npm version is run with both the --no-git-tag-version and --verbose flags, it logs an error if it cannot find a .git folder in the current folder. This error is very annoying for our automated build tooling and seems not relevant in the case that I don't want to update git.

Example of the error:

npm verb version Error: ENOENT: no such file or directory, stat 'C:\some-git-repo\dist\some-folder\.git'
npm verb version  error checking for .git { [Error: ENOENT: no such file or directory, stat 'C:\some-git-repo\dist\some-folder\.git']
npm verb version   stack:
npm verb version    'Error: ENOENT: no such file or directory, stat \'C:\\some-git-repo\\dist\\some-folder\\.git\'',
npm verb version   errno: -4058,
npm verb version   code: 'ENOENT',
npm verb version   syscall: 'stat',
npm verb version   path: 'C:\\some-git-repo\\dist\\some-folder\\.git' }
npm verb version not tagging in git

Expected output:

npm verb version not tagging in git

This PR just prevents the error from being logged. Of course another option would be to skip the statGitFolder altogether...

@woppa684 woppa684 requested a review from a team as a code owner October 7, 2019 13:44
@isaacs
Copy link
Contributor

isaacs commented Oct 7, 2019

Hm, this bit of code looks a bit odd to me. It probably needs a more thorough review at some point. You're right, it's a bit weird that we stat the git folder in that case just to log a verbose message saying that we're not doing anything with it. It would be good to keep the verbose log, but without the stat, if git-tag-version is turned off. It looks like even when git-tag-version is disabled, we still attempt to commit the change to git, later on in the _commit function.

I'm inclined to punt on this until we have some time to dig deeper into the issue. Would it be terrible to just run without --verbose? Do you need it for other things?

@woppa684
Copy link
Author

woppa684 commented Oct 8, 2019

That's indeed the route we're currently on, but in the mean time we want to be able to track this issue somewhere so that we can point at an issue or PR. I totally agree that this code looks a bit weird (not only my "fix" but _commit as well as you say).

What would be the proper way to make sure that this gets fixed at some point? I could dig a bit deeper myself and make a more complete solution of course.

@isaacs
Copy link
Contributor

isaacs commented Oct 8, 2019

We can just leave this open, it's fine. We may decide to take this change (or something like it), but I'd like to have a bit of time to track through the logic and make sure we're not causing other problems. (All too often I've found, when making a seemingly reasonable change in strange-seeming code, the results are strange and unexpected.)

@claudiahdz claudiahdz added the Needs Discussion is pending a discussion label Nov 12, 2019
@mikemimik mikemimik added Release 6.x work is associated with a specific npm 6 release semver:patch semver patch level for changes labels Nov 26, 2019
@mikemimik mikemimik added this to the Release 6.14.0 milestone Nov 26, 2019
mikemimik pushed a commit that referenced this pull request Dec 3, 2019
@mikemimik mikemimik closed this in ae7afe5 Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion is pending a discussion Release 6.x work is associated with a specific npm 6 release semver:patch semver patch level for changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants