-
Notifications
You must be signed in to change notification settings - Fork 60
Conversation
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.
In a way, it's kind of exciting that we've come far enough that we can do this! I think this is the right way forward.
It's probably worth adding a note in the changelog with a link to the new version. (That can be a separate PR once #1764 is also merged or we can do it later when we make the release.)
I've discussed with @mike-sul the possibility of writing some tests in the aktualizr repo to make sure we don't break the interfaces that aktualizr-lite depends on. (Or at least if we do, that it'll only be done with intention.) That doesn't need to be done now, but do you have any ideas of things that are missing in the tests that we be cover for that purpose?
I'm still interested in a proper TUF API at some point. I think it'd make all of our lives easier. I think it makes sense to write the tests in the aktualizr repo first, though, so that we have a good reference of the expectations. Then we can go about refactoring things to make the API at leisure. Does that make sense?
FYI I ran the larger test suite locally just to be sure and double-checked that no references to aktualizr-lite remain (except in the changelog). All looks good.
@@ -702,7 +702,7 @@ std::pair<bool, Uptane::Target> SotaUptaneClient::downloadImage(const Uptane::Ta | |||
|
|||
// Note: handle exceptions from here so that we can send reports and | |||
// DownloadTargetComple events in all cases. We might want to move these to | |||
// downloadImages but aktualizr-lite currently calls this method directly. | |||
// downloadImages |
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.
Is the fragment you removed no longer true?
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.
good catch. i've force pushed a fix.
5a572b6
to
ebcc2f1
Compare
Due to all the great work making libaktualizr more useful, we've been able to move aktualizr-lite to its own repository: https://github.com/foundriesio/aktualizr-lite Thanks for all the help getting this project going! Signed-off-by: Andy Doan <andy@foundries.io>
ebcc2f1
to
0718650
Compare
@doanac Yes, I think it makes sense to add some tests to the aktualizr repo that would mimic/emulate the lite client behavior. |
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.
LGTM.
Due to all the great work making libaktualizr more useful, we've been
able to move aktualizr-lite to its own repository:
https://github.com/foundriesio/aktualizr-lite
Thanks for all the help getting this project going!
Signed-off-by: Andy Doan andy@foundries.io