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.

Sign up for GitHub

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

Jump to bottom

Use global registration after renaming Smart Proxy Server #3033

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Use global registration after renaming Smart Proxy Server #3033

merged 1 commit into from
Aug 6, 2024

Conversation

maximiliankolb
Copy link
Contributor

@github-actions GitHub Actions
Copy link

github-actions bot commented May 16, 2024 •
edited

The PR preview for 423cc65 is available at theforeman-foreman-documentation-preview-pr-3033.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

Lennonka
Lennonka approved these changes May 20, 2024
View reviewed changes
Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

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

LGTM

guides/common/modules/proc_renaming-smart-proxy.adoc Show resolved Hide resolved
@maximiliankolb maximiliankolb force-pushed the rename_smart_proxy_global_registration branch from 2f65977 to dd62652 Compare May 21, 2024 07:10
ekohl
ekohl reviewed May 21, 2024
View reviewed changes
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 don't think global registration should be used to re-register. I'd much prefer a REX job to update the existing registration. Though I'm not 100% sure we have such a job already

@Lennonka

This comment was marked as outdated.

Sign in to view
@Lennonka Lennonka requested a review from ekohl May 24, 2024 15:57
@ShimShtein
Copy link
Member

I am creating a proper way to replace Foreman's CA in this pr: theforeman/foreman#10208

@maximiliankolb maximiliankolb force-pushed the rename_smart_proxy_global_registration branch from dd62652 to ab482a7 Compare August 6, 2024 09:44
maximiliankolb
maximiliankolb commented Aug 6, 2024
View reviewed changes
Copy link
Contributor Author

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

Rebased to "master", resolved all merge conflicts, and applied all suggestions.

guides/common/modules/proc_renaming-smart-proxy.adoc Outdated Show resolved Hide resolved
apinnick
apinnick requested changes Aug 6, 2024
View reviewed changes
Copy link
Contributor

@apinnick apinnick left a comment

Choose a reason for hiding this comment

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

Small style comment. Otherwise, LGTM.

guides/common/modules/proc_renaming-smart-proxy.adoc Outdated
----
endif::[]
endif::[]
. Re-register all hosts that are registered to your {SmartProxyServer}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Very small nit-pick. Our downstream guidelines say this about hyphenated prefixes:

"In most cases, do not use a hyphen to connect a prefix or suffix to a word. There are cases where a hyphen between a prefix and root word adds clarity. For more information, see ..."

We do not yet have a Vale rule to flag "pre-", "post-", "re-", etc. I think this might be a good rule to add. :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Correction: We do have a Vale rule. It does not include "re-register", so I created a PR to fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied.

@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Aug 6, 2024
@maximiliankolb maximiliankolb force-pushed the rename_smart_proxy_global_registration branch from ab482a7 to 423cc65 Compare August 6, 2024 11:25
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author Needs re-review labels Aug 6, 2024
@apinnick apinnick merged commit 62b12fb into theforeman:master Aug 6, 2024
8 of 9 checks passed
@maximiliankolb maximiliankolb deleted the rename_smart_proxy_global_registration branch August 6, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lennonka Lennonka Lennonka approved these changes

@apinnick apinnick apinnick approved these changes

@evgeni evgeni Awaiting requested review from evgeni

@ekohl ekohl Awaiting requested review from ekohl

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@maximiliankolb @Lennonka @ShimShtein @ekohl @apinnick