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 JWT install guide #3884

Merged
merged 29 commits into from
Aug 17, 2023
Merged

Update JWT install guide #3884

merged 29 commits into from
Aug 17, 2023

Conversation

jasonwilliams14
Copy link
Contributor

Proposed changes

This is an update to the JWT token method to install NGINX Ingress controller, providing a more detailed walkthrough.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • [x ] I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • [ x] I have checked that all unit tests pass after adding my changes
  • [ x] I have updated necessary documentation
  • [ x] I have rebased my branch onto main
  • [ x] I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@github-actions github-actions bot added the documentation Pull requests/issues for documentation label May 8, 2023
@codecov
Copy link

codecov bot commented May 8, 2023

Codecov Report

Merging #3884 (5efd999) into main (5af0ea5) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3884   +/-   ##
=======================================
  Coverage   51.95%   51.95%           
=======================================
  Files          59       59           
  Lines       16762    16762           
=======================================
  Hits         8708     8708           
  Misses       7755     7755           
  Partials      299      299           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link

@dreesf5 dreesf5 left a comment

Choose a reason for hiding this comment

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

I tested both via sources and manifest and both worked now. Small things to make it a bit more readable.

I'd also be more clear about how and when to use the namespace -n nginx-ingress.

@jasonwilliams14 jasonwilliams14 force-pushed the docs/jwt-token-update branch from 70f3e5b to cf62de5 Compare May 17, 2023 21:29
@jasonwilliams14 jasonwilliams14 force-pushed the docs/jwt-token-update branch 2 times, most recently from 06a10a5 to fcb0a54 Compare June 12, 2023 16:09
@brianehlert brianehlert added this to the v3.2.0 milestone Jun 20, 2023
@jasonwilliams14 jasonwilliams14 force-pushed the docs/jwt-token-update branch from b72a6a4 to 94ba7c2 Compare June 21, 2023 14:37
@github-actions
Copy link

github-actions bot commented Jun 21, 2023

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Manifest Files

@jasonwilliams14 jasonwilliams14 marked this pull request as ready for review June 21, 2023 14:47
@jasonwilliams14 jasonwilliams14 requested a review from a team as a code owner June 21, 2023 14:47
This commit makes some large changes in the layout, gramma and overall
phrasing of this installation guide. Many instances of the NGINX Ingress
Controller's name were changed, and where possible, text was made more
concise. Additionally, many links were added or renamed for explicit
ease in finding or understanding the context related to an instruction.
Copy link
Contributor

@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 have made numerous rewrites in this PR to improve the readability of the page: it will need an additional once-over at the minimum to ensure that the instructions are accurate.

@jasonwilliams14 jasonwilliams14 force-pushed the docs/jwt-token-update branch from 8110d02 to 737f13c Compare June 23, 2023 14:55
@jasonwilliams14 jasonwilliams14 force-pushed the docs/jwt-token-update branch from 1f54f6c to 1c48879 Compare June 23, 2023 20:59
@jasonwilliams14 jasonwilliams14 force-pushed the docs/jwt-token-update branch from 10adbff to 72d2957 Compare June 23, 2023 21:01
pre-commit-ci bot and others added 7 commits June 23, 2023 21:02
Signed-off-by: Jason Williams - NGIИX <[email protected]>
This branch has had multiple merge conflicts and force pushes,
completely erasing a number of large changes to the page to the point of
unintentionally removing entire sections.

This commit takes the last known good working version and adds the
changes back as a fresh new point for the branch.
Copy link
Contributor

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

This PR should be re-reviewed for accuracy (@jasonwilliams14 & @dreesf5 ) but I am happy with the state of the writing now.

Updated versions

Signed-off-by: Jason Williams - NGIИX <[email protected]>
Signed-off-by: Jason Williams - NGIИX <[email protected]>
@jasonwilliams14 jasonwilliams14 enabled auto-merge (squash) July 21, 2023 00:32
@lucacome lucacome requested a review from dreesf5 July 21, 2023 03:17
@lucacome lucacome dismissed dreesf5’s stale review July 22, 2023 00:44

the request was addressed

@brianehlert brianehlert modified the milestones: v3.2.0, v3.2.1 Aug 10, 2023
Signed-off-by: Jason Williams - NGIИX <[email protected]>
@jasonwilliams14 jasonwilliams14 requested a review from a team as a code owner August 17, 2023 17:57
@jasonwilliams14 jasonwilliams14 merged commit 1cc4428 into main Aug 17, 2023
@jasonwilliams14 jasonwilliams14 deleted the docs/jwt-token-update branch August 17, 2023 18:06
jasonwilliams14 added a commit that referenced this pull request Aug 17, 2023
(cherry picked from commit 1cc4428)
lucacome pushed a commit that referenced this pull request Aug 17, 2023
@brianehlert brianehlert modified the milestones: v3.2.1, v3.3.0 Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests/issues for documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants