Skip to content

Conversation

@diggy128
Copy link

@diggy128 diggy128 commented Jan 3, 2021

Modules should not be purged when completing scan without
explicit user consent. Instead select modules to purge from results.

After applied fixed an issue similar to #1534

Modules should not be purged when completing scan without
explicit user consent. Instead select modules to purge from results.
@diggy128 diggy128 force-pushed the 12.0_database_cleanup branch from ecd2525 to 6b5fdf4 Compare January 3, 2021 20:31
@thomaspaulb
Copy link

/ocabot merge minor

@StefanRijnhart
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-1978-by-StefanRijnhart-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Dec 13, 2023
Signed-off-by StefanRijnhart
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot
Copy link
Contributor

@StefanRijnhart your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-1978-by-StefanRijnhart-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@thomaspaulb
Copy link

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-1978-by-thomaspaulb-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 12, 2024
Signed-off-by thomaspaulb
@OCA-git-bot
Copy link
Contributor

@thomaspaulb your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-1978-by-thomaspaulb-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@thomaspaulb
Copy link

@diggy128 Interestingly, after your fix this test now fails, because it actually tests for a module already being purged during default_get.

@hbrunn Blame for that test leads to you; do you have any idea why it's there?

@StefanRijnhart
Copy link
Member

Yep, looks like purging modules immediately was a conscious change according to the explanation in this PR: #1316

@hbrunn
Copy link
Member

hbrunn commented Feb 13, 2024

thanks @StefanRijnhart, that's it. But by now we have the purge all button, and starting in version 14 they threw out this behavior anyways during migration.

@thomaspaulb
Copy link

So I modified tests as per discussion above, in #2857

Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants