-
Notifications
You must be signed in to change notification settings - Fork 2
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
Load OS version from boot partition for greater compatibility #37
Conversation
Hmm, making the tests pass is going to be tricky. The images shipped for the test suite are really old and don't have |
I would manually edit them and try to make them representative. They're not actually real images anyway, they're manually edited ones, because otherwise the repo would be massive. The other option ofc is that you could look at #26, but I suspect that's a fair bit more work. |
@pimterry, I've updated the images by mounting their partitions and copying the file from /etc/os-release, so it's as accurate as it can get. Tests pass now, thanks :) |
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.
Some small comments, but otherwise this looks like a nice simple fix.
Marking it as a major version bump so it's not picked up by really old CLIs which are probably used to provision old images which don't have os-release on the boot partition. (Please let me know if you think otherwise)
I think we can actually do this as a non-major bump, and it will improve things overall. The previous version was hacky and doesn't work in lots of cases, whilst this new version will work in a lot more cases. Also, if it doesn't work anywhere it's fine - we just configure for both, which is an acceptable result for old images, and shouldn't break anything anywhere. Happy to discuss, but personally I'd lean towards making this minor.
5234e71
to
2ca1238
Compare
This also removes the runtime dependency to balena-sdk and changes the public API of `configure` and `initialize` to require a device type manifest as an argument. Change-type: major
2ca1238
to
3f0f840
Compare
This also removes the runtime dependency to balena-sdk and changes the public API of
configure
andinitialize
to require a device type manifest as an argument.Change-type: major