fix: npm-version does not git-commit nor git-tag when package in subdirectory #4885
Conversation
yes, I'll add the test case. I'm trying to understand why other tests have failed. |
|
git is mocked in the tests - we don't use the package there. |
|
@fernandopasik I opened a PR into yours with what I hope are the right changes for the tests. I'm a GitHub noob, so this may have been a dumb way of doing it, but you should be able to see the ideas. If this stays idle for a few days, I can open up my own MR into this repo with both our changes. |
Mock find and test with subdirectories
|
@timothybonci I've merged your changes, thanks! |
|
How do we get the tests re-run? It looks like they only run once on PR open, but not on subsequent pushes, at least automatically. Are we waiting on a manual action by a maintainer? |
This reverts commit 2e40ec9.
|
I see failed tests from yesterday - it's not obvious to me why they're failing. I cloned the repo and can try and debug them, but I may not get to it until next week. |
Fixing my silly mistake. This fixes all but the last test.
Last test needs subdirectory evaluation
|
Just needed a good night's sleep. https://github.com/fernandopasik/cli/pull/2 fixes the tests. |
use same directory in the setup and the tests
|
@lukekarrys Is this waiting on you now? |
|
I see the new failures, thanks. |
|
I'm curious on how this will interact with workspaces, I believe it's important to make sure this change is not going to enable every |
|
According to this the design intent is to omit tagging when versioning workspaces. We can still create commits. I'll attempt to add a new condition before tagging that checks if we're in a workspace. This will require good tests, too. |
|
Having gotten more familiar with this, I'm leaning toward detecting workspaces, and then doing no tag and no commit. The convention we use in --git-tag-version false is "no tag, no commit". Inventing a new halfway commit with no tag for this use case doesn't seem wise. |
* fix: look parent dirs to detect git repo on version * Mock find and test with subdirectories * refactor: use let instead of var * style: use single quotes * Revert "refactor: use let instead of var" This reverts commit 2e40ec9. * use same directory in the setup and the tests Fixing my silly mistake. This fixes all but the last test. * Fix final test Last test needs subdirectory evaluation Co-authored-by: fernandopasik <fernando@pasik.com.ar>
|
I think I've got the workspace case covered now. https://github.com/fernandopasik/cli/pull/3 |
|
Something I thought about, but didn't implement - should --f always result in a commit, even in the workspaces case? It seems like the universal "let you do otherwise unwise things" but I worry about how overloaded it could get, turning into a footgun. I'd love to get feedback on this if it's desirable. |
|
@timothybonci almost all the work here is yours, please feel free to open a new PR with all the changes so I don't delay you. |
|
Thanks for starting this @fernandopasik. I've opened #5442 as a fresh start with my proposed changes. |
|
No problem, thanks for your contributions too. |
libnpmversion only checks one same dir if is git repo, instead use
git.findto check parents as wellReferences
fixes #2010