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

adr:Allow Ingress config snippet annotations #401

Merged
merged 1 commit into from Sep 14, 2022

Conversation

lucianvlad
Copy link
Contributor

ADR : Allow Ingress config snippet annotations

Copy link
Collaborator

@cristiklein cristiklein left a comment

Choose a reason for hiding this comment

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

LGTM, but given that I am part of the authorship group, I will not count myself towards approvers.

Can you please fix the pipeline issue?

Copy link
Contributor

@llarsson llarsson left a comment

Choose a reason for hiding this comment

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

I generally approve, but think it is also a bit strange to have a canned message in the ADR itself.

I would have preferred to have that outside of the architectural decision as a general principle. It provides good material for further reading and such, though.

@lucianvlad lucianvlad force-pushed the lucian/allow-ingress-config-snippet-annotations branch from 22c2d4b to 41ed7bb Compare September 13, 2022 14:13
@cristiklein
Copy link
Collaborator

@lucianvlad Forgot to mention, before merging, please regenerate the index, as described here.

@lucianvlad lucianvlad force-pushed the lucian/allow-ingress-config-snippet-annotations branch from 41ed7bb to b292f35 Compare September 14, 2022 13:09
@lucianvlad
Copy link
Contributor Author

@lucianvlad Forgot to mention, before merging, please regenerate the index, as described here.

Done just now

@lucianvlad lucianvlad merged commit 3b6e731 into main Sep 14, 2022
@lucianvlad lucianvlad deleted the lucian/allow-ingress-config-snippet-annotations branch September 14, 2022 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants