Skip to content
This repository has been archived by the owner on Dec 12, 2020. It is now read-only.

How to consolidate all update PRs into one PR? #938

Closed
ljharb opened this issue Oct 22, 2020 · 34 comments
Closed

How to consolidate all update PRs into one PR? #938

ljharb opened this issue Oct 22, 2020 · 34 comments

Comments

@ljharb
Copy link

ljharb commented Oct 22, 2020

I just enabled renovate on https://github.com/ljharb/list-exports, and promptly got 33 update PRs.

One issue is that it's trying to update all the package.jsons in my test fixtures folder - so I'm assuming I could add some config that would cause those to be ignored?

The other, though, is that I would expect one PR, not 33, for the initial update. Can that be configured?

@ljharb
Copy link
Author

ljharb commented Oct 23, 2020

Also, is there a way I could delay PR creation until the tests have succeeded?

@viceice
Copy link
Member

viceice commented Oct 23, 2020

How did you enabled renovate? Normally renovate would add a onboarding pr before opening other pr's.

Also i can't see a renovate.json config in your repo. So how do you configured renovate?

Why don't you have filled out issue template? There are essential questions you need to answer. Without them we aren't able to help you more.

@rarkins rarkins transferred this issue from renovatebot/renovate Oct 23, 2020
@rarkins
Copy link
Collaborator

rarkins commented Oct 23, 2020

I just enabled renovate on ljharb/list-exports, and promptly got 33 update PRs.

The onboarding PR is great for giving a preview of this. Also, if you were using the default settings in config:base it would have limited itself to 2 PRs per hour and maximum 20 concurrent to give you time to hopefully catch it.

One issue is that it's trying to update all the package.jsons in my test fixtures folder - so I'm assuming I could add some config that would cause those to be ignored?

See ignorePaths

The app by default proposes the config:base preset which includes :ignoreModulesAndTests. Looks like it would have ignored your tests folder by default if you were using it.

The other, though, is that I would expect one PR, not 33, for the initial update. Can that be configured?

Yes, any grouping can be configured manually, and there are a few presets available too. I don't recommend grouping unrelated major updates though. Try the preset group:allNonMajor

@viceice
Copy link
Member

viceice commented Oct 23, 2020

Sou really should have enabled onboarding. You can use organization level config to use a custom default onboarding config.

See https://docs.renovatebot.com/config-presets/#organization-level-presets

@ljharb
Copy link
Author

ljharb commented Oct 23, 2020

I'm sure there's many things in my config that I'm missing, that I should have :-) I generally prefer to explicitly define everything instead of relying on a preset, but I'll look into those, thanks.

If I update my .github config, what will happen to all the existing open PRs?

Re ignorePaths, what's the most concise in-repo renovate.json I'd need to extend my .github repo's config and add ignorePaths? If my .github config includes :ignoreModulesAndTests, how can i ensure i extend those default ignorePaths in individual repos and not override them?

@ljharb
Copy link
Author

ljharb commented Oct 23, 2020

I'm fine with an onboarding PR; how do i enable that?

@rarkins
Copy link
Collaborator

rarkins commented Oct 23, 2020

If I update my .github config, what will happen to all the existing open PRs?

At the end of each run, Renovate looks for any branches/PRs that shouldn't be there anymore. It then:

  • Renames the PR with " - autoclosed" suffix
  • Closes the PR
  • Deletes the branch

Re ignorePaths, what's the most concise in-repo renovate.json I'd need to extend my .github repo's config and add ignorePaths? If my .github config includes :ignoreModulesAndTests, how can i ensure i extend those default ignorePaths in individual repos and not override them?

ignorePaths is a non-mergeable field, meaning any configuration of it will override the previous config. i.e.

  • Good to define a default in your .github
  • If you need more in a particular repo (or less?) then you need to redefine the list

It's been implemented this way so that if 90% of the time you ignore tests folders it just works, but the other 10% you can simply override.

@rarkins
Copy link
Collaborator

rarkins commented Oct 23, 2020

I'm fine with an onboarding PR; how do i enable that?

Onboarding PRs were disabled in our backend as requested in #816, so I'd need to change that for you. However one challenge is that onboarding is normally accompanied with the requirement for a renovate.json file in each repo (or one of the supported filenames). i.e. normally the logic is:

  1. Is there a renovate config file?
  2. If not, is there an onboarding PR yet? If so then update it
  3. If not, then create the onboarding PR

In other words, any of your other existing repos may trigger an onboarding PR if we were to enable it, because they don't have a config file.

@ljharb
Copy link
Author

ljharb commented Oct 23, 2020

gotcha; sounds like that's not the path for me then, thanks. I'll explore how to use the builtin presets. Would this:

{
  "extends": ["github>ljharb/.github"],
  "ignorePaths": "whatever"
}

be what i need, or is there a simpler/more concise approach? i couldn't quickly find examples in the docs.

@rarkins
Copy link
Collaborator

rarkins commented Oct 23, 2020

Your repos already extend your preset by default, so you don't need to explicitly add extends again (but it also doesn't harm anything). For an ignorePaths example see https://docs.renovatebot.com/presets-default/#ignoremodulesandtests

@rarkins
Copy link
Collaborator

rarkins commented Oct 23, 2020

My recommendation is to add a good ignorePaths to your .github so that you get it by default in all your repos. Then you only need to add ignorePaths config to individual repos if you have a repo where you do want to update test or examples folders, or you have an abnormally named folder to ignore.

You also may want to add prConcurrentLimit and prHourlyLimit settings to your .github preset

@ljharb
Copy link
Author

ljharb commented Oct 24, 2020

I did a push to my .github dir changing the config, but it didn't seem to trigger anything on the list-exports repo. Do I need to rebase those PRs (or push to master so as to trigger that) for the new renovate settings to apply?

@viceice
Copy link
Member

viceice commented Oct 24, 2020

Yes, you will need a master commit to flush renovate cache, so it will reevaluate the whole config

@rarkins
Copy link
Collaborator

rarkins commented Oct 24, 2020

FYI the extract cache invalidates itself if the resolved repository config changes (e.g. renovate.json etc). So in this case because of @ljharb's unique setup of having it injected in the backend without repository config, changes to the upstream preset aren't detected. Will think if we can handle this without too much complexity

@ljharb
Copy link
Author

ljharb commented Oct 24, 2020

How would it normally work when a repo’s local config never changes, but the upstream config it extends changes?

@rarkins
Copy link
Collaborator

rarkins commented Oct 24, 2020

Actually, turns out it's not cache-related and will need a code fix so that we do an extra "resolve" step for the preset which is implicitly added. Working on it and will update you here once it's ready.

@rarkins
Copy link
Collaborator

rarkins commented Oct 25, 2020

@ljharb I think it's now working as intended. Please see ljharb/list-exports#40

There's now just one update pending and it requires dashboard approval first

@ljharb
Copy link
Author

ljharb commented Oct 25, 2020

Thanks! I'll leave that pending until renovatebot/renovate#4826 is implemented, since based on all of my projects' "engines" field, none of them should ever be prompted to update to rimraf 3 (or semver 7, or nyc > 11, or mocha > 3, etc).

@ljharb ljharb closed this as completed Oct 25, 2020
@rarkins
Copy link
Collaborator

rarkins commented Oct 25, 2020

That feature request is still in "needs requirements" stage because I don't think we really nailed down how it would work including edge cases. If you can help move it forward in that aspect, please feel free to comment

@ljharb
Copy link
Author

ljharb commented Oct 26, 2020

Thanks, I've commented - hopefully that helps.

@ljharb
Copy link
Author

ljharb commented Nov 6, 2020

The same thing has happened again, despite a dependency dashboard existing, and despite the config theoretically now ignoring, via preset, everything in "tests" directories.

@rarkins
Copy link
Collaborator

rarkins commented Nov 6, 2020

@ljharb are you able to point me to the repo/PR? And you clarify what "same thing" means exactly, because this issue took on a few topics

@rarkins rarkins reopened this Nov 6, 2020
@ljharb
Copy link
Author

ljharb commented Nov 6, 2020

https://github.com/ljharb/list-exports. I mean that I again received 30+ PRs to update dependencies in packages/tests/fixtures. The first set were autoclosed, which does re-title the PRs, so perhaps a recent change was made that cares about the PR title (which shouldn't matter, since users and renovate often rename them)?

@rarkins
Copy link
Collaborator

rarkins commented Nov 6, 2020

Never mind, I see you @'d me in a different repo.

Picking one PR at random, here's the files diff:

image

@ljharb
Copy link
Author

ljharb commented Nov 6, 2020

packages/tests/fixtures is meant to be ignored by my base renovate config, by pulling in a renovate preset that ignores **/tests/**, iirc. That ignore pattern is what triggered renovate to auto-close the first batch of 30+ PRs, so I'm surprised it would suddenly send new ones since it already detected it should ignore those files.

@ljharb
Copy link
Author

ljharb commented Nov 6, 2020

It's also notable that these PRs a) aren't mentioned in the dependency dashboard, and b) more of them were opened at once than my config's PR threshold should allow.

@rarkins
Copy link
Collaborator

rarkins commented Nov 6, 2020

so perhaps a recent change was made that cares about the PR title (which shouldn't matter, since users and renovate often rename them)?

There's nothing recently changed to my knowledge, and Renovate has cared about PR title since day 1, as that's how it checks if a PR has been raised before (and why it adds an - autoclosed suffix before autoclosing so that it doesn't block PRs in future if they're needed again).

In short: if you have a PR with same branch/title, that will block future PRs.

But in this case, that should be irrelevant, because the intention is that ignorePaths is being set to ignore tests? (you seem to agree based on your last comment, which popped up here before I'd finished typing)

@rarkins
Copy link
Collaborator

rarkins commented Nov 6, 2020

I am purely speculating right now, but it seems as if your preset is occasionally not picked up. So it goes from ignoring them and them being absent from the dashboard to suddenly and incorrectly thinking they all need creating. The logs hopefully can give us some indication if that's the case or not.

@ljharb
Copy link
Author

ljharb commented Nov 6, 2020

That all makes sense.

In particular, I never, on any repo, would want more than one unmerged PR created for the same dependency; I'd prefer to maximally reuse PRs, since every PR creates an eternal, undeleteable git ref (/pulls/1234).

@rarkins
Copy link
Collaborator

rarkins commented Nov 6, 2020

Are ok to suspend access for Renovate until I've worked out why that particular run misbehaved? I want to try to avoid it happening again. You could uninstall too but I think GitHub's option to suspend is most suitable.

@ljharb
Copy link
Author

ljharb commented Nov 6, 2020

Sure, i can just remove that repo for now - i only have a few enabled so far and that’s the only monorepo, and also the only one with text fixtures.

@rarkins
Copy link
Collaborator

rarkins commented Nov 6, 2020

It's not often that you spot a problem from literally line 1 of the logs.

The (good) job before:

{"level":30,"renovateVersion":"23.73.1","msg":"Repository started","time":"2020-11-06T15:29:52.986Z"}

The bad job that added all the PRs:

{"level":30,"renovateVersion":"23.49.7","msg":"Repository started","time":"2020-11-06T16:32:14.358Z"}

Seems we have a worker stuck on the old release and unfortunately this repo came up on the roulette wheel. I'll find and terminate it manually but need to think how to catch it automatically in future.

@ljharb
Copy link
Author

ljharb commented Nov 6, 2020

Gotcha, thanks :-) guess I'm just lucky.

Let me know when I should re-enable list-exports, and also continue onboarding new repos.

@rarkins
Copy link
Collaborator

rarkins commented Nov 6, 2020

You should be fine to re-enable that repo and continue now. We got particularly unlucky because I'd added some custom code at one point to enable your no-repo-config approach and unfortunately that worker was stuck on a release from earlier than that.

@rarkins rarkins closed this as completed Nov 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants