feat: Add --ignore-empty flag that will not bail command when no patchset is created #993
Conversation
kamilogorek
changed the title
AbhiPrasad
left a comment
AbhiPrasad
left a comment
There was a problem hiding this comment.
Think we should use ignore-empty
src/commands/releases.rs
Outdated
|
|
||
| if commits.is_empty() { | ||
| bail!("No commits found. Leaving release alone."); | ||
| // NOTE: Make it a default behavior on next major release instead? |
There was a problem hiding this comment.
Maybe we explicitly say TODO(v2): so it's easier to grep? Then we can make a decision on what we wanna do when we think about the next major bump.
There was a problem hiding this comment.
I don't know if that is a good idea. A release could consist of configuration changes through your environment. In those cases, I still want to have a release on Sentry. This could be a very common use case for everyone using the 12 factor app approach.
There was a problem hiding this comment.
Noted. Thanks for the feedback!
src/commands/releases.rs
Outdated
| will create a release with the default commits count (or the one specified with `--initial-depth`) \ | ||
| instead of failing the command.")) | ||
| .arg(Arg::with_name("allow-empty") | ||
| .long("allow-empty") |
There was a problem hiding this comment.
Not sure if allow-empty is the correct phrase to use here since --allow-empty in git adds commits with no content in them. In this case, this option refers to the fact that there is no commits to add to a release - I'm sure someone will get this confused when they try and use the allow-empty option in it's current state.
Maybe we should use ignore-empty. It won't conflict with git's --allow-empty (which may prevent future user misunderstandings), and ignore matches what is happening better than allow (updating the release is ignored).
There was a problem hiding this comment.
Valid reasoning. Changed
kamilogorek
changed the title
Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
Otherwise, there is no easy way to ignore this error and allow the CI to progress when triggering the same
set-commitcommand with no new commits. Simple|| trueis not sufficient, as it will silent all possible valid errors.Q: Not sure if we should call it
--allow-emptyor--ignore-emptyto be consistent with newly addedignore-missingflag.