search

spring-attic / bomr

Command-line tool for creating and updating a Maven bom
32 stars 5 forks source link

Provide options to suppress commits and issues on upgrade #17

Open dreis2211 opened 5 years ago

dreis2211 commented 5 years ago

Hi.

I was playing with bomr upgrade the other day and found it extremly helpful to have some sort of dry run option to review stuff even before it was committed. So this PR adds a --dry option to the upgrade command plus some related polishing. This new option could also help projects without GitHub integration for the time being. They could run the --dry upgrade and do the issue creation and commits manually.

I'm not super happy with the name of the new option, but dry was the closest to what it actually does. If you have a suggestion, I'm more than happy to change it.

Let me know what you think. Cheers, Christoph

wilkinsona commented 5 years ago

Thanks, @dreis2211.

Regarding --dry and inspired by Git (perhaps not the best source for command line design inspiration…), how about --dry-run?

More generally, I've wondered in the past about saving the choices that you make so that they can be applied later. The first thing that prompted this was running Bomr when GitHub was down which made the issue creation failed. There's no retry or recovery at the moment so this meant I had to go through the whole process again. While far from the same, such an option feels related to this. You could run in a save choices for later mode and then either apply them after review or discard them in you want to proceed manually. What do you think? Is there enough similarity that this could serve both purposes or would having both be better?

dreis2211 commented 5 years ago

@wilkinsona Thought about --dry-run as well and decided against it as I'm lazy and didn't want to type too much. ;-) But if the two of us already thought about it, this might be a sign to change it.

For me personally, the --dry(-run) option just serves the purpose of playing around and testing upfront without writing to the repository at all (either through commits or created issues).

The "save choices for later mode" - at least how I understand it - is sort of a two-step upgrade that still integrates with the repository eventually. You first make your choices with the first command. Maybe the first command already does the local changes. And later you can replay this choices by cleaning the table and going through the choices step by step again (and either applying or discarding them).

While I agree they have some overlap and could be used in a similar fashion for certain use-cases, I think they serve a different purpose:

Having that said, I think providing both options is the better choice.

Cheers, Christoph

wilkinsona commented 5 years ago

Thanks for your thoughts, @dreis2211. Let's go with two separate options and focus on a dry run here.

Having looked at the code and your description of how you may use the functionality, I wonder if dry or dry run is the right name. My expectation for a dry run is that it will leave things untouched and just tell me what would have happened if the command was run normally. That's not the case here as the bom is changed. Unless reset, this will then change the behaviour of a subsequent run. In short, I think the option probably needs a different name.

I wonder if the commits for individual updates are still useful in the testing scenario that you have described. They help to separate the upgrades and each can then be refined with fixup commits. Perhaps an option that suppresses issue creation would be sufficient?

dreis2211 commented 5 years ago

@wilkinsona Appreciate the feedback. What do you think of the following two options then:

This would control the two things that are involved in a more fine-grained way. Additionally, if --suppress-issues is specified alone, the commits won't mention Closes gh-12345

wilkinsona commented 5 years ago

Those two options sounds good to me. In the interests of brevity, how about:

dreis2211 commented 5 years ago

I like them 👍. Will change the PR accordingly. Probably on Friday though as there is a public holiday tomorrow in Germany.

dreis2211 commented 5 years ago

I've pushed the discussed changes. Funny enough during testing GitHub was not reachable for me for a short while, which made me apply the --no-issues option to both the milestone and label checks that are anyway redundant if no issues are created. That subsequently makes it possible to use bomr without a github repository (or at least improves the experience).