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

Add param for unsafe advisory conflict resolution #274

Merged

Conversation

@ekohl ekohl changed the title Add param for unsave advisory conflict resolution Add param for unsafe advisory conflict resolution Jan 31, 2023
templates/settings.py.erb Outdated Show resolved Hide resolved
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have checked this earlier, but this comes from pulp_rpm so it should also live in pulpcore::plugin::rpm. See 5e2bfd7 for an example of how a plugin-specific config option was added earlier.

In this case there is already some inline content so it probably makes sense to move it to a template. While most templates are ERB, I'd like to move to EPP. In 2e2d3ec I added an EPP template. Note how it uses epp() (with a context) instead of template() and a slightly different context.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a quick glance this looks ok and the test coverage should be sufficient.

@ekohl ekohl added the Enhancement New feature or request label Feb 2, 2023
@ekohl ekohl enabled auto-merge (rebase) February 2, 2023 16:24
@ekohl ekohl merged commit 9dcc9df into theforeman:master Feb 2, 2023
@m-bucher m-bucher deleted the add_advisory_conflict_resolve_option branch February 2, 2023 16:34
@m-bucher
Copy link
Contributor Author

m-bucher commented Feb 3, 2023

Thanks @ekohl for merging this.
I just remembered that I actually intended to set the default to true. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants