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

enabled poddisruptionbudget in case more than one broker instance is used #50

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

sherifkayad
Copy link
Contributor

enabled poddisruptionbudget in case more than one broker instance is used

@sherifkayad
Copy link
Contributor Author

I will have a look at the linting tomorrow morning first thing. It might have to do with my VSCode

@sherifkayad
Copy link
Contributor Author

@ChrisJBurns can you please have another look?

@ChrisJBurns
Copy link
Contributor

@sherifkayad Would you be able to bump the minor version as it's a new feature, I can approve and run the workflow once this is done 👍

@sherifkayad
Copy link
Contributor Author

@ChrisJBurns done! thanks for the feedback.

@sherifkayad
Copy link
Contributor Author

@ChrisJBurns can you help me identify why the linting is failing?

@sherifkayad
Copy link
Contributor Author

locally helm lint is succeeding
image

@ChrisJBurns
Copy link
Contributor

@sherifkayad I believe it's trailing whitespaces:

charts/pact-broker/values.yaml
  Error: 348:1 [trailing-spaces] trailing spaces

@sherifkayad
Copy link
Contributor Author

Can we re-check now?

@sherifkayad
Copy link
Contributor Author

sherifkayad commented Mar 29, 2023

@ChrisJBurns seems to be passing after my latest push 😅

ChrisJBurns
ChrisJBurns previously approved these changes Mar 30, 2023
Copy link
Contributor

@ChrisJBurns ChrisJBurns left a comment

Choose a reason for hiding this comment

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

lgtm

@ChrisJBurns
Copy link
Contributor

@sherifkayad One last thing, I've only just noticed, would you be able to sign your commit? The merging is blocked until this happens. The code changes are perfect, we just need to sign the commits

@sherifkayad
Copy link
Contributor Author

can you check now? again did a sign-off and forced pushing

@ChrisJBurns
Copy link
Contributor

@sherifkayad Sign-off is a great addition, but I was referring to commit signing. Apologies for being a pain, but it does add a layer of security to the commits.

…used

Signed-off-by: Sherif Ayad <sherif.k.ayad@gmail.com>
@sherifkayad
Copy link
Contributor Author

@ChrisJBurns also done ;) there you go

image

@ChrisJBurns ChrisJBurns merged commit f517f47 into pact-foundation:master Mar 30, 2023
@ChrisJBurns
Copy link
Contributor

@sherifkayad Thanks again! All merged :)

@sherifkayad
Copy link
Contributor Author

Sure thing! Thanks for your time reviewing as well.

@sherifkayad sherifkayad deleted the adding-pdb branch March 30, 2023 13:08
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.

2 participants