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

Add custom tag message #631

Merged
merged 15 commits into from
Dec 3, 2023

Conversation

LuisHenri
Copy link
Contributor

@LuisHenri LuisHenri commented Dec 4, 2022

Description

According to #558, I implemented a custom tag message for annotated tags.
Right now, the annotated tag is given as "tag_name" with its message as "tag_name" as well. It might be nice to have custom tag messages, for example, to add the features added on that tag.
The idea is a new argument to bump with the tag_message,

cz bump --annotated-tag-message "a message"
cz bump -atm "a message"

Checklist

  • Add test cases to all the changes you introduce
  • Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes

Expected behavior

Running the following commands will create an annotated tag with "a message" as the message of the tag.

cz bump --annotated-tag-message "a message"
cz bump -atm "a message"

Steps to Test This Pull Request

  1. Run cz bump -atm "a message"
  2. Run git tag -l --format='%(contents:subject)' <the-created-tag-name>
  3. Output should be 'a message'

Additional context

Based on Issue #558

Open questions:

  • Should I name the argument as --annotated-tag-message or as --annotated-tag-msg to keep it short?
  • For some reason, the Pipeline is not working correctly. I see that this is the case for other PRs as well. Is that really the case @Lee-W ?

@Lee-W
Copy link
Member

Lee-W commented Dec 29, 2022

Should I name the argument as --annotated-tag-message or as --annotated-tag-msg to keep it short?

I like --annotated-tag-message a bit more.

For some reason, the Pipeline is not working correctly. I see that this is the case for other PRs as well. Is that really the case @Lee-W ?

This should have been solved. Let me rebase your branch and see how it works.

@Lee-W
Copy link
Member

Lee-W commented Dec 29, 2022

Hi @LuisHenri ,tests/commands/test_bump_command.py::test_bump_changelog_command_commits_untracked_changelog_and_version_files failed even after rebasing. I might need your help to take a look at this issue. Thanks!

@codecov
Copy link

codecov bot commented Dec 30, 2022

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Files Coverage Δ
commitizen/__version__.py 100.00% <100.00%> (ø)
commitizen/bump.py 100.00% <100.00%> (ø)
commitizen/changelog.py 100.00% <100.00%> (+0.50%) ⬆️
commitizen/changelog_formats/asciidoc.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/base.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/markdown.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/restructuredtext.py 100.00% <100.00%> (ø)
commitizen/changelog_formats/textile.py 100.00% <100.00%> (ø)
commitizen/commands/bump.py 97.71% <100.00%> (+0.06%) ⬆️
commitizen/commands/changelog.py 99.07% <100.00%> (+0.13%) ⬆️
... and 25 more

📢 Thoughts on this report? Let us know!.

@LuisHenri
Copy link
Contributor Author

Hi @LuisHenri ,tests/commands/test_bump_command.py::test_bump_changelog_command_commits_untracked_changelog_and_version_files failed even after rebasing. I might need your help to take a look at this issue. Thanks!

Hey @Lee-W,
I fixed it. Seems like one of the tests was trying to run bump --annotated instead of bump --annotated-tag.

@frerksaxen
Copy link

I would really love to see this feature :)

@LuisHenri
Copy link
Contributor Author

@Lee-W is something else needed still?

@Lee-W
Copy link
Member

Lee-W commented Jul 21, 2023

Hi @LuisHenri , sorry for the late review; I'm out of bandwidth these days. But I planned to take a look at a few PRs tomorrow. If you can rebase this PR and mention me, I'll take a look tomorrow. Thanks!

@LuisHenri
Copy link
Contributor Author

@Lee-W Sorry for the delay. I'm done.

@Lee-W
Copy link
Member

Lee-W commented Jul 25, 2023

Hi @LuisHenri , instead of merging main branch, we prefer rebasing. You can do so through git rebase -i master

@LuisHenri LuisHenri force-pushed the feat/custom-tag-message branch from 3716b1b to c064be5 Compare July 25, 2023 18:43
@LuisHenri
Copy link
Contributor Author

@Lee-W Thanks for the advice! What about now? :)

@LuisHenri LuisHenri force-pushed the feat/custom-tag-message branch from 9d88ad3 to dead382 Compare July 25, 2023 18:53
@Lee-W
Copy link
Member

Lee-W commented Jul 26, 2023

@LuisHenri perfect! I'll try to take a deeper look this weekend.

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

Hi @LuisHenri , thanks for your contribution. the change looks good to me!

@woile I'm planning on merging this one these days. Let me know if you want to take a deeper look

tests/utils.py Outdated Show resolved Hide resolved
@Lee-W Lee-W added pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check and removed pr-status: wait-for-review issue-status: needs-triage labels Jul 29, 2023
@woile
Copy link
Member

woile commented Jul 30, 2023

Is it possible to add a message to a light tag? If not, should we throw an error to inform the user?

@Lee-W Lee-W added pr-status: wait-for-response and removed pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check labels Jul 30, 2023
@Lee-W
Copy link
Member

Lee-W commented Aug 20, 2023

@woile According to https://git-scm.com/book/en/v2/Git-Basics-Tagging, it seems we're not able to do that for a light tag

_opt = ""
if annotated:
_opt = f"-a {tag} -m"
if signed:
_opt = f"-s {tag} -m"
c = cmd.run(f"git tag {_opt} {tag}")

c = cmd.run(f'git tag {_opt} "{tag if _opt == "" or msg is None else msg}"')
Copy link
Member

Choose a reason for hiding this comment

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

@woile do you mean we should add a warning here to let the users know if they're using this feature, they're actually creating annotated tag?

To create a lightweight tag, don’t supply any of the -a, -s, or -m options, just provide a tag name:

https://git-scm.com/book/en/v2/Git-Basics-Tagging

not sure whether my understanding is correct

Copy link
Member

Choose a reason for hiding this comment

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

I see, maybe a comment there explaining that. I didn't know that if you provide a message it automatically becomes an annotated tag.

Copy link
Member

Choose a reason for hiding this comment

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

@woile I just added the comment and resolved the conflict. Could you please take a look? I think we're pretty close to merge this one :)

cc @noirbizarre

@Lee-W Lee-W requested a review from woile November 6, 2023 00:25
@frerksaxen
Copy link

@Lee-W git tag -m limits the tag annotation message to a one liner, correct? To have multiple lines you would need to allow multiple -m messages or provide a -F file. Or is it possible to line break with \n within the message string?

@Lee-W
Copy link
Member

Lee-W commented Dec 3, 2023

@Lee-W git tag -m limits the tag annotation message to a one liner, correct? To have multiple lines you would need to allow multiple -m messages or provide a -F file. Or is it possible to line break with \n within the message string?

Something like the following works

git commit -m "first line
second line
third line"

@Lee-W Lee-W added pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check and removed pr-status: wait-for-review labels Dec 3, 2023
@Lee-W
Copy link
Member

Lee-W commented Dec 3, 2023

@woile @noirbizarre I'm planning on merging this one today. Please let me know if you want to take a look. Thanks!

@Lee-W Lee-W merged commit 9a0164c into commitizen-tools:master Dec 3, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants