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

Change the rest of the docs to be alined with the dnf module #2289

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

Atroskelis
Copy link
Contributor

@Atroskelis Atroskelis commented Aug 21, 2024

Modified the rest of the documentation to align completely to the dnf module change and change any php81 referencences

Author checklist (Completed by original Author)

  • Good fit for the Rocky Linux project? Title and Author Metatags inserted ?
  • If applicable, steps and instructions have been tested to work
  • Initial self-review to fix basic typos and grammar completed

Rocky Documentation checklist (Completed by Rocky team)

  • 1st Pass (Document is good fit for project and author checklist completed)
  • 2nd Pass (Technical Review - check for technical correctness)
  • 3rd Pass (Detailed Editorial Review and Peer Review)
  • Final approval (Final Review)

…php81

Modified the rest of the documentation to align completely to the dnf module change and change any php81 referencences
Copy link

Test results for b82cb0b:

Number of broken URLs: 3

URL,RESULT,FILENAME
 http://www.neelc.org/,failed,guides/contribute/README.md
 https://certbot.eff.org/instructions?ws=apache&os=centosrhel8,failed,guides/security/generating_ssl_keys_lets_encrypt.md
 https://azuremarketplace.microsoft.com/en-us/marketplace/apps/resf.rockylinux-aarch64,failed,guides/cloud/migration-to-new-azure-images.md

@sspencerwire
Copy link
Contributor

sspencerwire commented Aug 21, 2024

@Atroskelis Thank you for your submission. Have you tested this with the LibreNMS procedure? This document was written some time ago, when RL 8 was the only Rocky Linux available. It was reviewed when 9 came out and some changes made, HOWEVER, the version of PHP at the time on RL 9, was 8.0 and 8.1 was not available. A quick check shows that this is still the case. The REMI repository was used to allow PHP 8.1, the minimum supported for LibreNMS. If you have not tested your changes on this procedure, then I'll need to close this PR. Let me know what you've done! Thank you again!

For more information, refer to https://docs.librenms.org/Installation/Install-LibreNMS/

@sspencerwire sspencerwire marked this pull request as draft August 21, 2024 13:38
@sspencerwire
Copy link
Contributor

@Atroskelis Thank you for your submission. Have you tested this with the LibreNMS procedure? This document was written some time ago, when RL 8 was the only Rocky Linux available. It was reviewed when 9 came out and some changes made, HOWEVER, the version of PHP at the time on RL 9, was 8.0 and 8.1 was not available. A quick check shows that this is still the case. The REMI repository was used to allow PHP 8.1, the minimum supported for LibreNMS. If you have not tested your changes on this procedure, then I'll need to close this PR. Let me know what you've done! Thank you again!

For more information, refer to https://docs.librenms.org/Installation/Install-LibreNMS/
It does appear (in RL 9) that PHP 8.1 is available as a module stream. The official documentation (the link shared above) has not been updated in some time, so it is possible that-with RL 9 at least-your changes would be OK. Provided you've tested them in the procedure, but you would still need to enable the module, which I don't see in your PR changes.

@Atroskelis
Copy link
Contributor Author

@sspencerwire The documentation was fixed with the official librenms documentation as a reference.

But the initial interest came today when i had a fresh Rocky 9 and wanted to install librenms and stumbled upon the broken rocky documenation, as half of it was using the dnf module thus replacing the command php81 with php and half was not. Thus the changes made.

From what i could guess it does seem that the Rocky9 Appsteam has php 8.1 and php 8.2.
image

Alas, it would still require fixing the documentation, and with this PR it makes it functional.

If you desire, the next time i change it, it will feature the native appstream package.

@sspencerwire
Copy link
Contributor

@sspencerwire The documentation was fixed with the official librenms documentation as a reference.

But the initial interest came today when i had a fresh Rocky 9 and wanted to install librenms and stumbled upon the broken rocky documenation, as half of it was using the dnf module thus replacing the command php81 with php and half was not. Thus the changes made.

From what i could guess it does seem that the Rocky9 Appsteam has php 8.1 and php 8.2. image

Alas, it would still require fixing the documentation, and with this PR it makes it functional.

If you desire, the next time i change it, it will feature the native appstream package.

Hi again @Atroskelis,
I just paged through the Rocky Linux documentation and I can't see where it is broken. It does not reference the newer PHP modules available in 9x, which is true. (Again, it was originally written for 8), but there is nothing that I can see that references anything other than the REMI php paths. So while the procedure is not perfect, it should work. So I'm trying to see what is broken. If you can point me to that, that would be great.
If you installed LibreNMS using different paths and procedures, having the entire procedure is helpful in editing the document. Changing the paths without changing the installation procedure for newer PHP from Appstream, would in my view, break the procedure entirely.
I guess what I'm saying is that if you are proposing these fixes, propose a few more to fix the PHP module install too.
Thank you!

@Atroskelis
Copy link
Contributor Author

Hello,

I shall try to explain

At the moment the Rocky librenms install docs uses the dnf to replace the default php install with the remi install. Initially the whole documentation used the php81 module which replaces all paths/executables/etc with php81 instead of php.

image

This means that the remi install will replace all php aspects.
Now, half of the documentation uses php paths and half uses php81 paths, hence this PR. If you use the dnf module to install basic php you do not have this path below.

image

Comparing to the official Librenms Install docs, which has thus:

image

@sspencerwire
Copy link
Contributor

Hello,

I shall try to explain

At the moment the Rocky librenms install docs uses the dnf to replace the default php install with the remi install. Initially the whole documentation used the php81 module which replaces all paths/executables/etc with php81 instead of php.

image

This means that the remi install will replace all php aspects. Now, half of the documentation uses php paths and half uses php81 paths, hence this PR. If you use the dnf module to install basic php you do not have this path below.

I'm sorry @Atroskelis, I don't see the "half" that uses anything. In fact, all I see is REMI php paths. I do not have an install of this at the moment to verify, so here's what I'm going to do: I'll merge this PR as is. At my earliest convenience, I'll roll a new install of LibreNMS using the changed instructions. Just these changes that you've put forward. If I can install step-by-step as an end user would AND the paths for the files are as you say, no harm, then I will concede that you are correct and something must have been wrong with my original instructions. I'm getting ready to head out of town for 4 days, and I don't want this sitting here languishing. Thank you again for your efforts!

image

Comparing to the official Librenms Install docs, which has thus:

image

@sspencerwire sspencerwire marked this pull request as ready for review August 23, 2024 01:10
@sspencerwire sspencerwire merged commit 6a73c39 into rocky-linux:main Aug 23, 2024
3 checks passed
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.

2 participants