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

Re-modularize the Monitoring Hosts section #2555

Conversation

mjivraja
Copy link
Contributor

@mjivraja mjivraja commented Oct 25, 2023

Re-modularize the Monitoring Hosts section in the Managing Hosts guide. This was decided in the #2502.

Please cherry-pick my commits into:

  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (planned Satellite 6.14)
  • Foreman 3.6/Katello 4.8
  • Foreman 3.5/Katello 4.7 (Satellite 6.13)
  • Foreman 3.4/Katello 4.6 (EL8 only)
  • Foreman 3.3/Katello 4.5 on EL7 & EL8 (Satellite 6.12 on EL8 only; orcharhino 6.4/6.5 on EL8 only)
  • Foreman 3.2/Katello 4.4 on EL7 & EL8
  • Foreman 3.1/Katello 4.3 on EL7 & EL8 (Satellite 6.11 EL7/8; orcharhino 6.3 on EL7/8)
  • We do not accept PRs for Foreman older than 3.1.

@github-actions
Copy link

github-actions bot commented Oct 25, 2023

The PR preview for c46b238 is available at theforeman-foreman-documentation-preview-pr-2555.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

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.

Generally, I think we can say just "Insights" instead of "Red Hat Insights" after the first fully-spelled occurence in the chapter title.
I'm suggesting a couple of initial thoughts on the module titles. (This includes also changing the file names and IDs.)
I will continue my review once these are agreed upon and incorporated.

@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Not yet reviewed labels Oct 30, 2023
@mjivraja mjivraja force-pushed the Remodularize-Insights-with-Hosts-sections-master branch from 4591cfb to 459226b Compare November 2, 2023 07:23
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Nov 2, 2023
@mjivraja mjivraja force-pushed the Remodularize-Insights-with-Hosts-sections-master branch from 459226b to 87158a9 Compare November 2, 2023 07:27
@mjivraja
Copy link
Contributor Author

mjivraja commented Nov 2, 2023

Oww no, why does the old guide Upgrading and Updating appear again in 6.14? 😲

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.

Please, try again. I'd like to move on to the really important stuff.

You did a good job with the remodularization, initially! I just thought we could take this opportunity to review and improve it in more detail.

@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Needs re-review labels Nov 2, 2023
@mjivraja mjivraja force-pushed the Remodularize-Insights-with-Hosts-sections-master branch from 87158a9 to 15f62e3 Compare November 3, 2023 13:21
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Nov 3, 2023
@mjivraja
Copy link
Contributor Author

mjivraja commented Nov 3, 2023

Apologies @Lennonka I overlooked things due to last-minute GA priorities. 😕

@mjivraja
Copy link
Contributor Author

mjivraja commented Nov 3, 2023

This time, the picture seems to be much bigger in linkchecker 😲

@Lennonka
Copy link
Contributor

Lennonka commented Nov 3, 2023

This time, the picture seems to be much bigger in linkchecker 😲

@mjivraja After re-running the linkchecker, the RHEL links are now alright. Please, rebase.

@mjivraja mjivraja force-pushed the Remodularize-Insights-with-Hosts-sections-master branch from 15f62e3 to eda8f73 Compare November 6, 2023 05:53
@mjivraja
Copy link
Contributor Author

mjivraja commented Nov 6, 2023

@mjivraja After re-running the linkchecker, the RHEL links are now alright. Please, rebase.

Addressed 👍

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.

Another piece of review.

@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Needs re-review labels Nov 7, 2023
@mjivraja mjivraja force-pushed the Remodularize-Insights-with-Hosts-sections-master branch from eda8f73 to 88425cb Compare November 20, 2023 11:30
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Nov 20, 2023
@mjivraja
Copy link
Contributor Author

Working on linkchecker

@mjivraja
Copy link
Contributor Author

Working on linkchecker

Will get fixed here #2596

@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Nov 21, 2023
@mjivraja mjivraja force-pushed the Remodularize-Insights-with-Hosts-sections-master branch from c19376c to 951d497 Compare November 28, 2023 13:54
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Nov 28, 2023
@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Needs re-review labels Nov 28, 2023
@mjivraja mjivraja force-pushed the Remodularize-Insights-with-Hosts-sections-master branch from 951d497 to 356f7c2 Compare November 29, 2023 05:46
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Nov 29, 2023
@mjivraja mjivraja force-pushed the Remodularize-Insights-with-Hosts-sections-master branch from 356f7c2 to 42fe542 Compare November 29, 2023 10:51
Copy link
Contributor

@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.

Please rebase to master and resolve the merge conflict.

@mjivraja mjivraja force-pushed the Remodularize-Insights-with-Hosts-sections-master branch from 42fe542 to c46b238 Compare December 7, 2023 14:10
@ShimShtein
Copy link
Member

LGTM from my side.

@Lennonka Lennonka merged commit ca90dcf into theforeman:master Dec 7, 2023
8 checks passed
@Lennonka
Copy link
Contributor

Lennonka commented Dec 7, 2023

Cherry-picked:

@mjivraja Conflicts on 3.7. Please file a separate PR.

FYI, please also prepare a separate PR for 3.6, because the hosts removal sync module isn't supposed to be there before 3.7 (#2502). Whether the rest of the modules is applicable to older branches is also questionable, please double check that with the current published docs.

@Lennonka
Copy link
Contributor

Lennonka commented Dec 7, 2023

@mjivraja On the other hand, do we necessarily have to cherry-pick this to 3.6 and older? For your consideration.
It would be nice to have, but it's additional work as mentioned above.

@mjivraja mjivraja deleted the Remodularize-Insights-with-Hosts-sections-master branch December 8, 2023 06:14
mjivraja added a commit to mjivraja/foreman-documentation that referenced this pull request Dec 8, 2023
@mjivraja
Copy link
Contributor Author

mjivraja commented Dec 8, 2023

@mjivraja Conflicts on 3.7. Please file a separate PR.

FYI, please also prepare a separate PR for 3.6, because the hosts removal sync module isn't supposed to be there before 3.7 (#2502). Whether the rest of the modules is applicable to older branches is also questionable, please double check that with the current published docs.

Thanks for heads-up, @Lennonka

I completely overlooked the #2502 and I agree that this PR should not be cherry-picked below 3.7.

Here we go for 3.7: #2634

ianballou pushed a commit to ianballou/foreman-documentation that referenced this pull request Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants