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

systematically call updateIngressStatus #6148

Merged
merged 1 commit into from
Jan 22, 2020
Merged

Conversation

mpl
Copy link
Collaborator

@mpl mpl commented Jan 9, 2020

What does this PR do?

This PR moves the call to updateIngressStatus so that it is always called, whereas previously it would not have been called in .e.g some error cases. While we were at it, also added a small refactor to unindent some code.

Motivation

This should have done (or at least discussed) in #6121, but IMO the review got rushed.

More

  • [ ] Added/updated tests
  • [ ] Added/updated documentation

Additional Notes

@ldez ldez added this to To review in v2 via automation Jan 10, 2020
@mpl mpl changed the title small refactor for readability systematically call updateIngressStatus Jan 20, 2020
@ldez ldez added this to the next milestone Jan 20, 2020
Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM
:shipit:

Copy link
Contributor

@timoreimann timoreimann left a comment

Choose a reason for hiding this comment

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

LGTM 👍

(Travis failed, but the reason seems unrelated -- 404 with Helm.)

@traefiker traefiker merged commit c24e74e into traefik:master Jan 22, 2020
v2 automation moved this from To review to Done Jan 22, 2020
@mpl mpl deleted the updatestatus branch April 13, 2022 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v2
Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants