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

Warn about unhandled arguments #2641

Merged
merged 3 commits into from
May 9, 2022
Merged

Warn about unhandled arguments #2641

merged 3 commits into from
May 9, 2022

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented May 3, 2022

Proposed changes

I was trying to use the ingress binary and unfortunately gave it the wrong first argument. Because of how flag.Parse() works, all of the arguments were ignored and the error I got was very confusing.

Checklist

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

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

@brianehlert brianehlert requested a review from lucacome May 3, 2022 14:09
cmd/nginx-ingress/main.go Outdated Show resolved Hide resolved
@lucacome lucacome self-assigned this May 6, 2022
@lucacome lucacome requested review from a team, haywoodsh and jjngx May 6, 2022 16:35
@codecov-commenter
Copy link

Codecov Report

Merging #2641 (1d4991c) into main (420aa9a) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2641      +/-   ##
==========================================
- Coverage   53.44%   53.41%   -0.03%     
==========================================
  Files          52       52              
  Lines       14702    14706       +4     
==========================================
- Hits         7857     7855       -2     
- Misses       6585     6589       +4     
- Partials      260      262       +2     
Impacted Files Coverage Δ
cmd/nginx-ingress/main.go 6.81% <0.00%> (-0.05%) ⬇️
internal/k8s/configuration.go 95.47% <0.00%> (-0.39%) ⬇️
...ternal/k8s/appprotect/app_protect_configuration.go 86.74% <0.00%> (+0.57%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Co-authored-by: Luca Comellini <[email protected]>
@lucacome
Copy link
Member

lucacome commented May 9, 2022

Merging this, thanks @jsoref !

@lucacome lucacome merged commit 22d3d7f into nginx:main May 9, 2022
@jsoref jsoref deleted the unhandled-args branch May 9, 2022 11:27
@ciarams87 ciarams87 added the chore Pull requests for routine tasks label Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Pull requests for routine tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants