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

Does npm version really need tag ? #21

Closed
binsee opened this issue Apr 8, 2022 · 5 comments · Fixed by #24
Closed

Does npm version really need tag ? #21

binsee opened this issue Apr 8, 2022 · 5 comments · Fixed by #24
Labels
enhancement New feature or request

Comments

@binsee
Copy link
Member

binsee commented Apr 8, 2022

We seem to rarely use tags, and npm version in the push hook will create new tags by default. And tags only exist in the local record and will not be uploaded to the remote repo. The tags are displayed in the git log, which is annoying.

So I think tag should not be created in push hook.
The --no-git-tag-version flag can be added to npm version to skip creating tags.

image

@huan
Copy link
Member

huan commented Apr 9, 2022

Thanks for raising this issue!

The tag can be removed because we are not using it at all.

Please feel free to create a PR to disable the tag creation.

The --no-git-tag-version flag can be added to npm version to skip creating tags.

@huan huan added the enhancement New feature or request label Apr 9, 2022
binsee added a commit to binsee/chatie-git-scripts that referenced this issue Apr 9, 2022
@binsee
Copy link
Member Author

binsee commented Apr 9, 2022

[DOCS] git-tag-version=false does not commit
npm/cli#3710

--no-git-tag-version doesn't work as I understand it, and it causes commits not to be executed.

So we can only abandon this option and use delayed execution git tag -d to delete the tag.

@huan huan closed this as completed in #24 Apr 9, 2022
huan pushed a commit that referenced this issue Apr 9, 2022
* feat: no git tag version
Fixes:  #21

* fix: 🐛 --no-git-tag-version does not commit

see npm/cli#3710

* 0.7.4
@huan
Copy link
Member

huan commented Apr 9, 2022

Merged.

Please let me know if it works as expected or not.

Thank you very much for your improvement!

@binsee
Copy link
Member Author

binsee commented Apr 9, 2022

Please let me know if it works as expected or not.

Yes, it works.

binsee@localhost > ~/CodeWork/sidecar > test3 > git push --dry-run            

> [email protected] lint
> npm-run-all lint:es lint:ts


> [email protected] lint:es
> eslint --ignore-pattern fixtures/ "src/**/*.ts" "tests/**/*.ts"


> [email protected] lint:ts
> tsc --isolatedModules --noEmit

v1.0.30
Deleted tag 'v1.0.30' (was e1626d8)
To github.com:binsee/sidecar.git
   6e813d3..335daca  test3 -> test3

____ _ _        ____            _
/ ___(_) |_     |  _ \ _   _ ___| |__
| |  _| | __|    | |_) | | | / __| '_ \
| |_| | | |_     |  __/| |_| \__ \ | | |
\____|_|\__|    |_|    \__,_|___/_| |_|

____                              _ _
/ ___| _   _  ___ ___ ___  ___  __| | |
\___ \| | | |/ __/ __/ _ \/ _ \/ _^ | |
___) | |_| | (_| (_|  __/  __/ (_| |_|
|____/ \__,_|\___\___\___|\___|\__,_(_)






 ### Npm version bumped and pushed by inner push inside hook pre-push ###"
 -- vvvvvv outer push will be canceled, don't worry, not bug :) vvvvvv --"



Failed to exec pre-push hook script
error: failed to push some refs to 'github.com:binsee/sidecar.git'
binsee@localhost > ~/CodeWork/sidecar > test3 > git push --dry-run
Everything up-to-date

@huan
Copy link
Member

huan commented Apr 9, 2022

Glad to know, cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants