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

Fixes #37496 - RHEL registration template prioritizes paramaters #10188

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

aburhinox
Copy link
Contributor

@aburhinox aburhinox commented May 29, 2024

Fixes #37496

This changes the priority on subman_org to use host_parm if present, then use katello generated org. This will allow users to override the katello generated org through a host_parm, and if not present continue to work as today with the katello generated org.

@aburhinox aburhinox marked this pull request as draft May 30, 2024 00:57
@aburhinox aburhinox marked this pull request as ready for review May 30, 2024 00:58
@aburhinox aburhinox changed the title Fixes #37496 - RHEL registration template ignores host_parm subscription_manager_org Fixes #37496 - RHEL registration template prioritizes paramaters May 30, 2024
Copy link
Contributor

@stejskalleos stejskalleos left a comment

Choose a reason for hiding this comment

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

IMO, it does make sense. We should allow users to override the output of the template by host parameters, but I'll leave the final ACK to @nofaralfasi

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.

@nofaralfasi is on PTO for this week, but it makes sense to me too. Shall we just merge it now?

Copy link
Contributor

@nofaralfasi nofaralfasi left a comment

Choose a reason for hiding this comment

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

LGTM.

@ekohl
Copy link
Member

ekohl commented Jul 1, 2024

@aburhinox could you rephrase the the commit message to fit our convention? I also don't know why CI didn't run, so updating your commit may also solve that.

@aburhinox
Copy link
Contributor Author

Thanks for taking a look at this. I believe I followed the commit message guidance I found https://theforeman.org/handbook.html but it's still failing the automated tests. Can anyone help point out what is wrong with the commit message? I have the redmine number, added the short description, and have slightly longer description below, not sure why it's failing.

@stejskalleos
Copy link
Contributor

4_commits
Hi,
It looks like you made a mistake while cleaning up the commit message. Right now, there are four commits instead of one.
How to fix it:

Make sure you have the latest develop:

  • git checkout develop
  • git pull upstream develop

Then rebase your branch and cleanup commits:

  • git checkout your-branch
  • git rebase -i develop, keep only this line:
reword d94fd9d Fixes #37496 

(remove all other commit lines)
  • Save and exit editor
  • Rename the commit message to "Fixes #ID - Your message"
  • Save and exit editor

Check changes:

  • git diff develop

After you check that everything is correct:

  • git push -f

Changed priority on subman_org to use host_parm if present, then use katello generated org
@aburhinox
Copy link
Contributor Author

thanks @stejskalleos, that did the trick. it looks like it's in good condition to review now.

@stejskalleos stejskalleos merged commit 55d2a0d into theforeman:develop Jul 29, 2024
51 of 53 checks passed
@stejskalleos
Copy link
Contributor

Thanks @aburhinox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants