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.

Sign up for GitHub

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

Jump to bottom

feat(argo-events): add priorityClassName and podLabels options #610

Merged
merged 1 commit into from
Apr 26, 2021
Merged

feat(argo-events): add priorityClassName and podLabels options #610

merged 1 commit into from
Apr 26, 2021

Conversation

t3mi
Copy link
Contributor

@t3mi t3mi commented Mar 4, 2021 •
edited
Loading

Checklist:

  • I have updated the chart version in Chart.yaml following Semantic Versioning.
  • Any new values are backwards compatible and/or have sensible default.
  • I have followed the testing instructions in the contributing guide.
  • I have signed the CLA and the build is green.
  • I will test my changes again once merged to master and published.

Changes are automatically published when merged to master. They are not published on branches.

@oliverbaehler
Copy link
Contributor

@t3mi Thanks for your contribution, Sorry that it took some time for someone to review it. Could you pull the latest master and resolve potential conflicts and bump to a new version? After I can merge it :)

@t3mi t3mi requested a review from oliverbaehler as a code owner April 26, 2021 11:08
@t3mi
Copy link
Contributor Author

t3mi commented Apr 26, 2021

@oliverbaehler done.

oliverbaehler
oliverbaehler suggested changes Apr 26, 2021
View reviewed changes
Copy link
Contributor

@oliverbaehler oliverbaehler left a comment

Choose a reason for hiding this comment

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

I also recommend using with, since you achieve the same affect but don't have to write the same value path twice. If you could accept the suggestions for the sake of consistency 🚨

charts/argo-events/templates/sensor-controller-deployment.yaml Outdated Show resolved Hide resolved
charts/argo-events/templates/sensor-controller-deployment.yaml Outdated Show resolved Hide resolved
charts/argo-events/templates/eventsource-controller-deployment.yaml Outdated Show resolved Hide resolved
charts/argo-events/templates/eventsource-controller-deployment.yaml Outdated Show resolved Hide resolved
charts/argo-events/templates/eventbus-controller-deployment.yaml Outdated Show resolved Hide resolved
charts/argo-events/templates/eventbus-controller-deployment.yaml Outdated Show resolved Hide resolved
oliverbaehler
oliverbaehler approved these changes Apr 26, 2021
View reviewed changes
Copy link
Contributor

@oliverbaehler oliverbaehler left a comment

Choose a reason for hiding this comment

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

lgtm

@oliverbaehler oliverbaehler merged commit 72f7192 into argoproj:master Apr 26, 2021
terrych0u pushed a commit to terrych0u/argo-helm that referenced this pull request Sep 7, 2021
@t3mi @terrych0u
feat(argo-events): add priorityClassName and podLabels options (argop…
6dacac2 
…roj#610)

Signed-off-by: t3mi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliverbaehler oliverbaehler oliverbaehler approved these changes

@jbehling jbehling Awaiting requested review from jbehling

@VaibhavPage VaibhavPage Awaiting requested review from VaibhavPage

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@t3mi @oliverbaehler