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

Update nginx.md #604

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Update nginx.md #604

wants to merge 4 commits into from

Conversation

RickX56
Copy link

@RickX56 RickX56 commented Nov 27, 2021

By submitting this pull request, I confirm the following:
please fill any appropriate checkboxes, e.g: [X]

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes, and have included unit tests where possible.
  • [] I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • [] It is compatible with the EUPL 1.2 license
  • [] I have squashed any insignificant commits. (git rebase)

Please make sure you Sign Off all commits. Pi-hole enforces the DCO.


What does this PR aim to accomplish?:
A detailed description, screenshots (if necessary), as well as links to any relevant GitHub issues

This modification makes the Web Interface working with NGINX.

How does this PR accomplish the above?:
A detailed description (such as a changelog) and screenshots (if necessary) of the implemented fix

Edit /etc/nginx/nginx.conf to contain the following in the http section:
gzip on;
gzip_min_length 1000;
gzip_proxied expired no-cache no-store private auth;
gzip_types text/plain application/xml application/json application/javascript application/octet-stream text/css;
include /etc/nginx/conf.d/*.conf;

What documentation changes (if any) are needed to support this PR?:
A detailed list of any necessary changes

Add section before section 7 with
"Edit /etc/nginx/nginx.conf to contain the following in the http section:
gzip on;
gzip_min_length 1000;
gzip_proxied expired no-cache no-store private auth;
gzip_types text/plain application/xml application/json application/javascript application/octet-stream text/css;
include /etc/nginx/conf.d/*.conf;
"

(This is my first ever contribution to GitHub, so I hope the formatting is okay).
Thank you for developing such great tool!

  • You must follow the template instructions. Failure to do so will result in your pull request being closed.
  • Please respect that Pi-hole is developed by volunteers, who can only reply in their spare time.

@netlify
Copy link

netlify bot commented Nov 27, 2021

Deploy Preview for pihole-docs ready!

Name Link
🔨 Latest commit ab62b98
🔍 Latest deploy log https://app.netlify.com/sites/pihole-docs/deploys/67bb9fc25fa4790008545484
😎 Deploy Preview https://deploy-preview-604--pihole-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@yubiuser
Copy link
Member

Thanks for your contribution.
Could you elaborate a bit more, why you added this section?

The automatic test fail due to styling:

docs/guides/webserver/nginx.md:85:76 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
docs/guides/webserver/nginx.md:86 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```bash"]
docs/guides/webserver/nginx.md:94:1 MD029/ol-prefix Ordered list item prefix [Expected: 8; Actual: 9; Style: 1/2/3]
docs/guides/webserver/nginx.md:100:1 MD029/ol-prefix Ordered list item prefix [Expected: 9; Actual: 8; Style: 1/2/3]
docs/guides/webserver/nginx.md:106:1 MD029/ol-prefix Ordered list item prefix [Expected: 10; Actual: 9; Style: 1/2/3]
docs/guides/webserver/nginx.md:112:1 MD029/ol-prefix Ordered list item prefix [Expected: 11; Actual: 10; Style: 1/2/3]
docs/guides/webserver/nginx.md:118:1 MD029/ol-prefix Ordered list item prefix [Expected: 12; Actual: 11; Style: 1/2/3]
docs/guides/webserver/nginx.md:124:1 MD029/ol-prefix Ordered list item prefix [Expected: 13; Actual: 12; Style: 1/2/3]

@RickX56
Copy link
Author

RickX56 commented Dec 15, 2021

@yubiuser So I added the gzip command to limit the file size while retaining the original file mode, ownership and timestamp.
The main change is include /etc/nginx/conf.d/*.conf;. Before adding this line the web UI was not working (nothing was displayed at all in the web page). I found this fix on https://wiki.archlinux.org/title/Pi-hole#Nginx_instead_of_Lighttpd

@yubiuser
Copy link
Member

The tests still faill here

docs/guides/webserver/nginx.md:85:76 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
docs/guides/webserver/nginx.md:86 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```bash"]
docs/guides/webserver/nginx.md:106:1 MD029/ol-prefix Ordered list item prefix [Expected: 10; Actual: 9; Style: 1/2/3]
docs/guides/webserver/nginx.md:112:1 MD029/ol-prefix Ordered list item prefix [Expected: 11; Actual: 10; Style: 1/2/3]
docs/guides/webserver/nginx.md:118:1 MD029/ol-prefix Ordered list item prefix [Expected: 12; Actual: 11; Style: 1/2/3]
docs/guides/webserver/nginx.md:124:1 MD029/ol-prefix Ordered list item prefix [Expected: 13; Actual: 12; Style: 1/2/3]

yubiuser
yubiuser previously approved these changes Dec 16, 2021
@yubiuser yubiuser added the PR: Approved Open Pull Request, Approved by required number of reviewers label Dec 16, 2021
@dschaper dschaper requested a review from DL6ER January 11, 2022 09:15
@PromoFaux PromoFaux requested review from DL6ER and removed request for DL6ER February 24, 2022 15:47
@clayoster clayoster mentioned this pull request Nov 19, 2024
1 task
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@RickX56
Copy link
Author

RickX56 commented Feb 22, 2025

Hello, I'm not sure what I have to do to resolve the conflicts.
Could you advise, please ?

@yubiuser
Copy link
Member

In your local git repo, fetch the upstream. Then you can either do a rebase of this branch on master, or merge the current master into this branch.

@RickX56
Copy link
Author

RickX56 commented Feb 23, 2025

Thank you for help Yubiuser !
It should be good now

@yubiuser
Copy link
Member

I think you forgot to push your changes to github

Add section before section 7 with "Edit /etc/nginx/nginx.conf to contain the following in the http section:
gzip            on;
gzip_min_length 1000;
gzip_proxied    expired no-cache no-store private auth;
gzip_types      text/plain application/xml application/json application/javascript application/octet-stream text/css;
include /etc/nginx/conf.d/*.conf;
"

This modification makes the Web Interface working with NGINX.

(This is my first contribution to GitHub, so I hope the formatting is okay).
Updated to correct numbering
Copy link
Contributor

Conflicts have been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Approved Open Pull Request, Approved by required number of reviewers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants