Skip to content

Conversation

@dkleinF5
Copy link

@dkleinF5 dkleinF5 commented Dec 2, 2025

Proposed changes

Checklist

Before sharing this pull request, I completed the following checklist:

Footnotes

  1. Potentially sensitive information includes personally identify information (PII), authentication credentials, and live URLs. Refer to the style guide for guidance about placeholder content.

@dkleinF5 dkleinF5 requested a review from a team as a code owner December 2, 2025 16:01
@github-actions github-actions bot added documentation Improvements or additions to documentation product/waf Issues related to F5 WAF for NGINX labels Dec 2, 2025
Copy link
Member

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

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

Generally LGTM: please ensure your commit messages (in-branch or merge) adhere to our Git conventions as outlined in the checklist part of every PR, which expect Conventional Commits formatting.

@github-actions
Copy link

github-actions bot commented Dec 2, 2025

Deploy Preview will be available once build job completes!

Name Link
😎 Deploy Preview https://frontdoor-test-docs.nginx.com/previews/docs/1510/

@dkleinF5 dkleinF5 changed the title fixed nginx conf docs: update subrequest section Dec 2, 2025
Copy link
Member

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

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

image

LGTM still, but you should reconsider how you have changed the headings.

Heading level 4 does not generate in the table of contents, meaning that they require the user to scroll to find them.

I suggest removing the "Additional..." third level subheadings, and renaming the subrequest "Example" heading into a "General example" to ensure that the headings are uniformly consistent from that point on.

@y82
Copy link
Contributor

y82 commented Dec 3, 2025

Perhaps it's worth adding the slice module here as well.
It'd be also good to align the values in config examples

@dkleinF5 dkleinF5 requested review from ADubhlaoich and y82 December 3, 2025 14:14
Copy link
Member

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

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

LGTM: I can merge this.

Copy link
Member

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

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

I just noticed you added more commits since yesterday.

It is very unclear to me when to review this PR because commits keep being added: I do not understand what the definition of done is meant to look like.

@dkleinF5 dkleinF5 marked this pull request as draft December 4, 2025 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation product/waf Issues related to F5 WAF for NGINX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants