crate-ci / cargo-release

Cargo subcommand `release`: everything about releasing a rust crate.
Apache License 2.0
1.31k stars 109 forks source link

Use actual tag string in confirmation message of tag step #652

Open cmyr opened 1 year ago

cmyr commented 1 year ago

Thanks for the crate, I'm finding this very useful!

I just ran into a tiny thing: When running cargo release tag, I get a confirmation message that shows the crates and versions that are going to be tagged, but it doesn't show the actual final tag text; e.g for me it said,

Tag
  crate-one 0.3.0
  crate-two 0.1.0
? [y/N] 

And so I wasn't sure what the actual tags would be, and had to go digging through the source. Maybe it make sense instead to do,

Tag
  crate-one-v0.3.0
  crate-two-v0.1.0
? [y/N] 
lolbinarycat commented 3 months ago

this is also relevant if you use a different tag format, such as not prefixing version tags with "v". "cargo release tag" doesn't print the name of the tag that will be created, which is pretty bad UX.

epage commented 3 months ago

As proposed, this is only relevant for cargo release tag which seems like a fairly niche case. I expect most people will be tagging through cargo release and showing the tags there feels odd. For cargo release there is a lot of validation one needs to do for release configuration. Most of this is one-time. The best way to do it is with cargo release -vv in dry-run mode.

If we showed tags, that would also made the bald v1.0.0 tag confusing.

So I'm having a hard time understanding why this is important and why this wouldn't cause more problems.

lolbinarycat commented 3 months ago

If we showed tags, that would also made the bald v1.0.0 tag confusing.

it's already somewhat confusing. if i'm trying to make the release tagged "1.0.0" and not "v1.0.0", seeing "v1.0.0" everywhere is a bit worrying.

the "push" step actually does show the tag name, but with no feedback to say it's the tag name, not the version string.

 Pushing Pushing 1.1.3, trunk to origin
epage commented 3 months ago

The push step is naming refs that are being pushed.

lolbinarycat commented 3 months ago

you closed my pull request because of "unanswered questions", despite the fact that thus far, noone has even used a question mark. nonetheless, i'll try to answer what i think you might've meant: "why is this needed"

let's say someone accidentally creates src/release.toml and puts their desired tag format in there. they run cargo release tag and get no output. they run C-x b release.toml, make sure the file is saved, and run cargo release again. it still shows vX.Y.Z, and they conclude that that line of output must just always use the v prefix like every other line of code does. they run --execute and go do something else, satisfied with their work. a bit later they check their repo and realize it pushed the wrong tag name. they now have to run back to their laptop, figure out how to delete/rename a tag, do that, manually create the tag, hope they haven't broken any links...

why this wouldn't cause more problems

what problems could adding one more line of output cause? the only possible confusion i can see this creating is the present tense "tagging" making people think it's actually being performed even though it's a dry run... but that applies equally to all the other output too, so clearly that's not actually a concern?

epage commented 3 months ago

Could you help me understand the use case for

what problems could adding one more line of output cause? the only possible confusion i can see this creating is the present tense "tagging" making people think it's actually being performed even though it's a dry run... but that applies equally to all the other output too, so clearly that's not actually a concern?

Note that #796 is different than what was proposed in this issue which is what I am discussing. imo #796's style of output feels like a net negative to me. So my hesitation over the proposed output of being confusing of what the tags are associated with still stands.

lolbinarycat commented 3 months ago

Why you are running cargo release tag enough to notice and care?

why does it exist if people aren't supposed to use it? "why are you using this software for its intended purpouse" is kinda an odd question and i'm not sure how to answer it.

Why verbose output isn't sufficient?

  1. not everyone will think to use --verbose, or even that it exists (not every program has it)
  2. more importantly, it outputs a lot of information completly irrelevant to the user, such as details of regex building. all of this extra output makes it difficult to spot the actual relevant output.

Note that https://github.com/crate-ci/cargo-release/pull/796 is different than what was proposed in this issue which is what I am discussing

true, it doesn't implement exactly the proposed solution, but it aims to solve the same problem (not being sure which tags will be created), and it solves it in a way that is more general, fixing the issue for cargo release tag and for cargo release. and unlike the proposed solution, it also prints in --dry-run mode, allowing you to catch issues earlier in your workflow.

imo https://github.com/crate-ci/cargo-release/pull/796's style of output feels like a net negative to me. So my hesitation over the proposed output of being confusing of what the tags are associated with still stands.

would you prefer "Tagging CRATENAME vX.Y.Z as TAGNAME"? i can adjust it to that pretty easily. if you have any other specific concerns about the format of the output, i'm sure i can address those too.

epage commented 3 months ago

why does it exist if people aren't supposed to use it? "why are you using this software for its intended purpouse" is kinda an odd question and i'm not sure how to answer it.

I'd recommend engaging assuming others are engaging in good faith. If there is something that seems out of place, consider what you might not understand and ask questions rather than being antagonizing.

In general, the subcommands in cargo release were intended for more rare use cases like recovering from failures or advanced use cases of combining things in ways that don't fit our strict release model. The emphasis that is being put on cargo release tag in this issue makes it sound more important to users than it was intended and I'm wanting to better understand how people are using it so we can better prioritize and better design to people's needs.

not everyone will think to use --verbose, or even that it exists (not every program has it)

I can understand that not everyone will think to but that is a challenge with cargo release is that there are so many parts to a release process that need one-time confirmation and would be overwhelming that that is the best solution we have at the moment.

more importantly, it outputs a lot of information completly irrelevant to the user, such as details of regex building. all of this extra output makes it difficult to spot the actual relevant output.

If those are a problem, we can have a separate issue for better focusing the verbose output.

it solves it in a way that is more general, fixing the issue for cargo release tag and for cargo release

Keep in mind, a lot of what you are saying is not unique to tags and we have to consider how these concerns scale to the whole workflow.

lolbinarycat commented 3 months ago

I can understand that not everyone will think to but that is a challenge with cargo release is that there are so many parts to a release process that need one-time confirmation and would be overwhelming that that is the best solution we have at the moment

it seems like you are suggesting that there is so much info that is important to check on the first release, but mostly irrelevant on following releases, that showing all of it by default would make the tool unusable. i do not think that is the case.

I can understand that not everyone will think to but that is a challenge with cargo release is that there are so many parts to a release process that need one-time confirmation and would be overwhelming that that is the best solution we have at the moment

i'm sure there is other info that would be nice to know, but i haven't found myself missing anything besides tag names. if other issues come up in the future, they can be addressed in the future, unless you can enumerate them in the present.

epage commented 3 months ago

Could you please speak to why cargo release tag became of such interest?

it seems like you are suggesting that there is so much info that is important to check on the first release, but mostly irrelevant on following releases, that showing all of it by default would make the tool unusable. i do not think that is the case.

i'm sure there is other info that would be nice to know, but i haven't found myself missing anything besides tag names. if other issues come up in the future, they can be addressed in the future, unless you can enumerate them in the present.

For myself, I run --verbose for replacements and dependent-version much more often than I do for tags.

lolbinarycat commented 3 months ago

Could you please speak to why cargo release tag became of such interest?

the scope of the changes i want is a bit more than just cargo release tag, it applies to the tagging step, no matter how it is run. it seems quite strange to have one of your main phases produce no output.

would you rather i create a new issue requesting exactly the functionality i already implemented?

epage commented 3 months ago

That is good to know that you care more generally. The framing of this issue was about cargo release tag. Your initial post on this was scoped to cargo release tag. I don't think a separate issue is needed yet until we have a better idea of where this is going and then it could be split off if needed.

it applies to the tagging step, no matter how it is run. it seems quite strange to have one of your main phases produce no output.

Its a balancing act of showing the relevant information to the user without overwhelming such that they can't find relevant information. Generally, the push step status will show the tags so listing tags separately doesn't add anything. That was done before we added support for "step" subcommands (e.g. tag) and we didn't take into account the exact output for running each step, in particular that

$  cargo release tag
warning: aborting release due to dry run; re-run with `--execute`

doesn't say what was done. At least it doesn't produce no-output which would have been much worse as it would have looked like it was doing something but didn't.

While the original issue focused on the confirmation prompt for cargo release tag -x and other discussion has shifted to cargo release, from what I've seen so far, I'm most sympathetic to cargo release tag. If there is clarification to help me understand why I should be more sympathetic to other cases, I'm open to it. As-is, I'm wanting to give this some time to stew before deciding on cargo release tag

lolbinarycat commented 3 months ago

Generally, the push step status will show the tags so listing tags separately doesn't add anything.

it adds clarity. it says "these are the tags that will be created". the push step lists all of the refs that will be pushed, some or all of which may already exist. additionally, it makes the output easier to skim: if i'm looking at all the output, and i want to know what tags will be created, i have to know to look for the line starting with "Push", with is a bit unintuitive.