-
Notifications
You must be signed in to change notification settings - Fork 104
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
Fixing issue 271 #300
Fixing issue 271 #300
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ajanth97 I have a few minor changes. after that, this looks good. thanks!
@arschles thank you for the suggestions. I will fix the DCO issue tomorrow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Ajanth <ajanth1997@gmail.com>
kedacore#298) * chore: Provide configuration for automatically closing inactive issues Signed-off-by: GitHub <noreply@github.com> * Align with core Signed-off-by: GitHub <noreply@github.com> * feature instead of feature-request Signed-off-by: GitHub <noreply@github.com> Signed-off-by: Ajanth <ajanth1997@gmail.com>
Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth <ajanth1997@gmail.com>
Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth <ajanth1997@gmail.com>
Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth <ajanth1997@gmail.com>
Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth <ajanth1997@gmail.com>
Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com> Signed-off-by: Ajanth <ajanth1997@gmail.com>
ee82b08
to
01c9683
Compare
@arschles thanks for the new suggestion too. Fixed the DCO issue |
} | ||
newCM := routingConfigMap.DeepCopy() | ||
if err := routing.SaveTableToConfigMap(table, newCM); err != nil { | ||
lggr.Error(err, "couldn't save new routing table to ConfigMap", "configMap", routing.ConfigMapRoutingTableName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ajanth97 can you outdent this so it lines up with the line below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
It also looks like you'll need to fix a test: https://github.com/kedacore/http-add-on/runs/3977788275?check_suite_focus=true#step:4:75 |
Signed-off-by: Ajanth <ajanth1997@gmail.com>
Signed-off-by: Ajanth <ajanth1997@gmail.com>
Thanks for this @ajanth97! |
Signed-off-by: Ajanth ajanth1997@gmail.com
Provide a description of what has been changed
Checklist
README.md
docs/
directoryFixes #271