-
Notifications
You must be signed in to change notification settings - Fork 524
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
Remove cruft not required in new releases #774
Conversation
019b12a
to
61f631d
Compare
This push changes "settings.example" to "settings.motd" per discussion with bcressey above. #774 (comment) Updating testing in the description now. |
It's useful to have an example setting so that users can see it in documentation and follow along, learning the API by actually making calls, without worrying about breaking their system. We were using hostname and timezone for this, but those sounded like real settings (and were originally intended to be real settings) so it was confusing. motd is a "real" setting in that it updates /etc/motd, but that's very low risk and only exposed to the user in any case. Plus, with low risk, it's fun to see an actual impact on your system.
61f631d
to
6069812
Compare
This push just fixes a typo in the updated commit message; sorry! |
This will be a breaking release, so we clear the migrations list.
This push adds a commit that updates Release.toml, removing the migrations that were removed in this PR, and updating the version to reflect why. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😀
🕵🏻♂️ |
Testing done:
Updated unit tests pass.
I launched an instance and was able to run a pod OK. The migrations tarball is produced, but empty, as expected.
I saw the new default motd in /etc/motd. I changed the setting, committed, and saw the change in /etc/motd.
Regarding the removal of the migrator interface workaround, I tested the new interface didn't need it in #644, and the old interface doesn't exist anymore with this PR.