refactor(install): simplify InstallMethod methods, pt. 2

[rami3l]

Part of rust-lang/goals#736.

This is a simple follow-up of #5081. As usual, only inlining and deduplication are involved.

However, I might not further simplify this API from here, because there will be more stuff to be returned from .install() later on, notably the stale object to be garbage collected...

[djc]

Here's one reason I'm not a big fan of your commit message style.

[rami3l]

@djc I agree I could have been more succinct in composing my commit messages. I've just shortened them to be below 70 letters each. If you would like to suggest a general style and/or a length limit for the first line (for example, you may want to keep the most important messages upfront as you have done with the function parameters so that it may be easier for you to review on the particular client you are using), please feel free to let me know :)

That said, I'm still surprised by the bad presentation here. It looks like this on my mobile:

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[djc]

@djc I agree I could have been more succinct in composing my commit messages. I've just shortened them to be below 70 letters each. If you would like to suggest a general style and/or a length limit for the first line (for example, you may want to keep the most important messages upfront as you have done with the function parameters so that it may be easier for you to review on the particular client you are using), please feel free to let me know :)

I think if you're going to stick with "Conventional Commits"-style messages, you should never use a two-segment scope style. Personally, I don't like the conventional commits style, here's a recent blog post arguing against it:

https://sumnerevans.com/posts/software-engineering/stop-using-conventional-commits/

[rami3l]

Personally, I don't like the conventional commits style

@djc Thanks for voicing your opinions on this. For now I don't think it's practical to enforce a commit message style or format, but I will try my best to avoid long prefixes and keep the message length to 50 chars or so.

here's a recent blog post arguing against it:

sumnerevans.com/posts/software-engineering/stop-using-conventional-commits

I have already read that blog post so I see your point, but I still have my reservations on this topic. From my personal experience, this is sometimes helpful with changelogs and backports (especially when I want to distinguish unexpected breakages from refactoring patches and intended breakages from features).

[djc]

@djc Thanks for voicing your opinions on this. For now I don't think it's practical to enforce a commit message style or format, but I will try my best to avoid long prefixes and keep the message length to 50 chars or so.

I think keeping the message length to 50 chars is actually an anti-pattern -- it's good to make use of the full 70 or so chars for the first line, but not by filling it with repetitive boilerplate.

I have already read that blog post so I see your point, but I still have my reservations on this topic. From my personal experience, this is sometimes helpful with changelogs and backports (especially when I want to distinguish unexpected breakages from refactoring patches and intended breakages from features).

I understand that part, but IIRC the convention allows the simple use of ! to denote incompatible changes -- so those don't actually need to take up a lot of space?

[rami3l]

but IIRC the convention allows the simple use of ! to denote incompatible changes -- so those don't actually need to take up a lot of space?

No it doesn't, I agree as I've said above that my previous prefix was too long.

I think keeping the message length to 50 chars is actually an anti-pattern -- it's good to make use of the full 70 or so chars for the first line, but not by filling it with repetitive boilerplate.

I am thinking about like around 50 without the prefix and in total it should be around 70. So I guess we are on the same page here.