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

Do settings as a pre-install hook #564

Merged
merged 6 commits into from Mar 19, 2019
Merged

Conversation

rickducott
Copy link

Avoids race condition where settings gets created by the pod first, causing the install to fail. Also includes a small refactor to minimize code dupe.

@soloio-bot
Copy link

Issues linked to changelog:
#562

@rickducott rickducott added the work in progress signals bulldozer to keep pr open (don't auto-merge) label Mar 19, 2019
marcogschmidt
marcogschmidt previously approved these changes Mar 19, 2019
Copy link
Contributor

@marcogschmidt marcogschmidt left a comment

Choose a reason for hiding this comment

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

Looks good!

@rickducott
Copy link
Author

/kick

@rickducott
Copy link
Author

Only change since @marcogschmidt reviewed was rearranging some tests (removed tests that were broken and never run, added tests that are relatively fast to start testing install command). Not happy with coverage for the new functions but it is hard to test, regression tests should provide sufficient assurance that install still works however.

@rickducott rickducott removed the work in progress signals bulldozer to keep pr open (don't auto-merge) label Mar 19, 2019
@soloio-bulldozer soloio-bulldozer bot merged commit d30f7a2 into master Mar 19, 2019
@soloio-bulldozer soloio-bulldozer bot deleted the settings-pre-install branch March 19, 2019 19:18
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

3 participants