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 nginx Rockon Documentation #488 #489

Merged
merged 3 commits into from
Oct 7, 2024

Conversation

Hooverdan96
Copy link
Member

@Hooverdan96 Hooverdan96 commented Oct 4, 2024

Fixes #488.

This pull request's proposal

Add a write-up for the nginx Rockon
This PR is based on the work that Rockstor community forum user Stevek compiled.

Changes implemented:
Add nginx Rockon documentation

  • nginx document initial release (using Stevek's version, updating formatting, some minor changes)
  • link into Rockon overview page (section of Rockons with write-ups)
  • add to Authors file

Checklist

  • With the proposed changes no Sphinx errors or warnings are generated.
  • I have added my name to the AUTHORS file, if required (descending alphabetical order) - in this case Stevek (or kaufstev on github).

Hooverdan96 and others added 2 commits October 4, 2024 11:41
Add nginx Rockon documentation
- nginx document initial release
- link into Rockon overview page
- add to Authors file
alphanumeric descending
@Hooverdan96
Copy link
Member Author

There are a couple of link checks that are failing not related to this addition. Will check it out eventually and attempt to fix them. Also a few permanent relocations of targets seems to have occurred recently.

fix formatting and http to https link
Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

Thanks to Steve Kaufman on our forum for main authoring and instigating this Rock-on write-up. And to @Hooverdan96 for seeing this work through to submission here on GitHub.

This is a super write-up with some really nice clarification points: particularly pertaining to the 'overlap' that can be hard to grasp regarding the docker instance and the base OS (openSUSE) instance of Nginx. Nice. As such it should server as a good write-up for those in need of a local development instance of Nginx.

I have some modifications I'd like to see regarding repetition (intra, and inter doc section), and some company links I would rather not have in-place. Plus a country reference that is inappropriate given Nginx's open-source world wide contributor base etc. Plus a few trivial capitalisation etc. But overall this is good to go and my modifications can wait and undergo a review process of their own. Given @Hooverdan96 (core-contributor) has already prepared this, lets get this in and published so we can get more eyes on what folks might want to see improvemnets wise.

Thanks again to Steve & @Hooverdan96 for all the work in getting this to this stage. And my apologies for not being able to contribute to that effort. But I do plan to present a PR in time with my suggested changes here: mainly regarding a simplification re repetition of what is already written-up re Rock-ons really: but essentially just stylistic.

I'll get this merged and published shortly so others can more easily contribute as they see fit.

@phillxnet
Copy link
Member

My main outstanding issue here really is the confusing inclusion of systemd references to the OS Nginx instance via systemd. This entire section can be removed as we are only wanting to give instructions on the Rock-on itself. But I do see the point. And again we can have this discussion via any number of follow-up PR. But I'm keep to get Steve's & @Hooverdan96 effort published as it is easily at the same level as any other section of our docs that could be picked out :). I think we also should avoid first person references such as " I will explain their use below.". Our project is always a team effort: but again this is a trivial modification that is not worth delaying further the publishing of what looks like a useful contribution for perspective web developers who need a local instance of Nginx to prove their proposed deployments/configurations.

N.B. I also had a functional issue with the f5 link (maintenance indicated). But I'd like to remove that link in time anyway.

@phillxnet
Copy link
Member

We have some rockstor-core changes pending re Rock-ons summary/info screens that will also impact an image o two here (and mixed png/jpg). But again no show-stoppers: just cleaner presentation to come, and some normalisation we need to establish across our docs anyway.

@phillxnet phillxnet merged commit ea9b4e8 into rockstor:master Oct 7, 2024
2 of 3 checks passed
@phillxnet
Copy link
Member

PR product PRODUCTION published.

@Hooverdan96 Hooverdan96 deleted the 488_niginx_Rockon_Document branch October 8, 2024 15:13
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.

Document usage of nginx Rockon
2 participants