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

Support multiple commits per branch/PR #7288

Open
rarkins opened this issue Sep 15, 2020 · 11 comments
Open

Support multiple commits per branch/PR #7288

rarkins opened this issue Sep 15, 2020 · 11 comments
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:feature Feature (new functionality)

Comments

@rarkins
Copy link
Collaborator

rarkins commented Sep 15, 2020

What would you like Renovate to be able to do?

Support an option to separate commits instead of having only one.

Did you already have any implementation ideas?

There are a few challenges to solve:

  1. Changing our logic to commit multiple times in a branch instead of just at the end. Currently we build up a long list of changed files and then perform a single commit once all updates are applied.

  2. Knowing when to split and when to join. For example, separate each updated npm dependency into a separate commit, and then the combined lock file update to yet another afterwards? Even if all the initial commits on their own would fail tests and only the last commit is a truly valid state? And what about postUpdateOptions like de-duping and tidying - combine them into the previous commit or separate them?

  3. How to detect if a branch is modified or not. Currently we only fetch to depth 2, but depth of our commits could potentially become dozens or hundreds in theory. Perhaps it's still good enough to simply check the gitAuthor of the latest commit and assume the branch is unmodified if the author is "us"?

Are there any workarounds or alternative ideas you've tried to avoid needing this feature?

The current approach of a single commit works very well.

Is this a feature you'd be interested in implementing yourself?

Maybe

@rarkins rarkins added type:feature Feature (new functionality) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others labels Sep 15, 2020
@rarkins rarkins added the status:requirements Full requirements are not yet known, so implementation should not be started label Jan 12, 2021
@sathieu
Copy link

sathieu commented Jan 14, 2023

@rarkins I'm very interrested by this.

Configuration-wise, this could be implemented with a commitGroupName similar to groupName. commitGroupName would be a sub-category of groupName.

Implementation-wise, I have no idea if this is hard (not a TypeScript dev).

@rarkins rarkins added status:requirements Full requirements are not yet known, so implementation should not be started and removed status:requirements Full requirements are not yet known, so implementation should not be started labels Oct 1, 2023
@juancarlosjr97
Copy link
Contributor

This issue is welcoming PR to try to address this feature?

@rarkins
Copy link
Collaborator Author

rarkins commented Mar 8, 2024

It's tagged as status:requirements because there's a lot of unknowns here so research/advised first. PRs are welcome if that's rhetorical easiest way to explore, but will only be merged if the result is maintainable!

@juancarlosjr97
Copy link
Contributor

That sounds like a fun weekend ahead! Thank you @rarkins !

@rarkins
Copy link
Collaborator Author

rarkins commented Mar 9, 2024

FYI I believe that this would be a pre-requisite: #21558

But unfortunately we needed to revert the feature twice due to regression errors

@juancarlosjr97
Copy link
Contributor

juancarlosjr97 commented Mar 26, 2024

I have not had that much time to work on this one, it is particularly important to me, so keep it on top of my list!

I was thinking that perhaps instead of doing a commit per change which becomes complex, because I have not given too much thought to all the supported technologies by Renovate but initially thinking of node and the package-lock.json, thinking that perhaps is easier to put all the changes in a single commit but comma-separated, not a big fan of my idea straight away but putting here to see what everyone think.

It breaks the principle of atomic commits but it is just easier to say a big commit with a lot of changes and keep one commit and the other things we have discussed here as keeping track of n-commits and so on go away.

@juancarlosjr97
Copy link
Contributor

Any thoughts on the above @rarkins , not the best solution but would be a lot better as a first change to start showing all the changes :)?

@rarkins
Copy link
Collaborator Author

rarkins commented Apr 11, 2024

It's already possible to populate the commit message body with a list of updates (https://docs.renovatebot.com/configuration-options/#commitbodytable) and does not solve this issue

@juancarlosjr97
Copy link
Contributor

Oh, I did not know that that was brilliant! It solves my issue :)

Based on that, I will shift my focus from this ticket as https://docs.renovatebot.com/configuration-options/#commitbodytable does what I need!

@dnephin
Copy link

dnephin commented May 23, 2024

I've started using renovate with grouping (matchPackagePatterns, groupName configuration, with the go datasource) and separate commits would help a lot.

Occasionally, especially with the k8s.io dependencies we get a group of dependency updates where one of them is backwards incompatible in some way. When that happens it's a lot of work to figure out which one is the problem. We've already added a special case for k8s.io direct dependencies, but the problem still occurs when k8s.io modules are indirect dependencies of something else.

If each dependency update was done as a separate commit, running go mod tidy each time, it would be much easier to detect and resolve these problems. Either the run of go mod tidy each time should identify the commit, or we can use git-bisect with go test to find the problem and simply revert the one commit.

@jftanner
Copy link
Contributor

This is a featured I'd very much like to have, due to the interaction with semantic-release. It generates one entry in the release notes for each commit merged.

For example: I currently have my package rules set up to combine all the patch updates together into one PR. This makes Renovate less noisy, but results in a commit message like Upgrade: Update patch dependencies. Then, when semantic-release publishes it, my release notes just say:

Upgrade:

  • Update patch dependencies

If Renovate did individual commits for each dependency, then I could get one PR but my release notes would still list all the individual dependencies that were updated. Best of both worlds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:feature Feature (new functionality)
Projects
None yet
Development

No branches or pull requests

5 participants