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

boot: split MakeBootable implementations into their own file #7973

Merged
merged 4 commits into from Jan 10, 2020

Conversation

anonymouse64
Copy link
Member

Also some drive-by tweaks to add doc-comments and remove underscores from some variable names.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
Not adding one to NameAndRevision because it will be ripped out in short order.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
@anonymouse64 anonymouse64 added Simple 😃 A small PR which can be reviewed quickly UC20 labels Jan 9, 2020
Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Thank you!

@@ -150,7 +144,7 @@ type bootState interface {
// revisions retrieves the revisions of the current snap and
// the try snap (only the latter might not be set), and
// whether the snap is in "trying" state.
revisions() (snap, try_snap *NameAndRevision, trying bool, err error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I wonder why the static checks have not warned about those. In any case, thanks for the fix!

Copy link
Member Author

Choose a reason for hiding this comment

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

well tbh the thing that is complaining for me is go-lint, which is more strict than go vet, so run-checks won't complain about it. just a little nit pick thing that I don't think we need to block PR's on, so we don't need to add anything to run-checks

boot/boot.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

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

thank you, some small nitpicks

boot/boot.go Outdated Show resolved Hide resolved
boot/makebootable.go Outdated Show resolved Hide resolved
@pedronis pedronis changed the title boot.go: split makebootable family into its own file boot: split makebootable family into its own file Jan 9, 2020
* Add "." to end of doc-comments for consistency
* Move MakeBootable to right after BootableSet for simplicity

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
@anonymouse64 anonymouse64 force-pushed the tweak/split-boot-go-make-bootable branch from 5f1e418 to beaf0a8 Compare January 9, 2020 16:23
@anonymouse64
Copy link
Member Author

whoops I had a typo in one of my commit messages and thought I had fixed it before pushing up first time, evidently not and accidentally force-pushed

@anonymouse64
Copy link
Member Author

The uc20 regression test suite failed because the reboot request timed out:

2020-01-09 17:25:05 Error preparing google:ubuntu-core-20-64:tests/regression/ : kill-timeout reached after jan091643-562499 (google:ubuntu-core-20-64:tests/regression/) reboot request
travis_time:end:8e8789c2:start=1578588762152602050,finish=1578590705926324402,duration=1943773722352
travis_time:start:207d2894
2020-01-09 17:25:05 Restoring google:ubuntu-core-20-64:tests/regression/...
travis_time:end:207d2894:start=1578590705926613140,finish=1578590705926682727,duration=69587
travis_time:start:207d2895
2020-01-09 17:25:05 Error restoring google:ubuntu-core-20-64:tests/regression/ : read tcp 10.20.0.136:35072->35.231.6.253:22: read: connection reset by peer
travis_time:end:207d2895:start=1578590705926613141,finish=1578590705926708536,duration=95395
travis_time:start:207ed513
2020-01-09 17:25:05 Restoring google:ubuntu-core-20-64:tests/regression/...
travis_time:end:207ed513:start=1578590705926722835,finish=1578590705926752152,duration=29317
travis_time:start:207ed514
2020-01-09 17:25:05 Error restoring google:ubuntu-core-20-64:tests/regression/ : read tcp 10.20.0.136:35072->35.231.6.253:22: read: connection reset by peer
travis_time:end:207ed514:start=1578590705926722836,finish=1578590705926764678,duration=41842
travis_time:start:207f9797
2020-01-09 17:25:05 Restoring google:ubuntu-core-20-64...
travis_time:end:207f9797:start=1578590705926772631,finish=1578590705926788773,duration=16142
travis_time:start:207f9798
2020-01-09 17:25:05 Error restoring google:ubuntu-core-20-64 : read tcp 10.20.0.136:35072->35.231.6.253:22: read: connection reset by peer
travis_time:end:207f9798:start=1578590705926772632,finish=1578590705926797762,duration=25130
2020-01-09 17:25:05 Discarding google:ubuntu-core-20-64 (jan091643-562499)...
2020-01-09 17:25:06 Successful tasks: 3341
2020-01-09 17:25:06 Aborted tasks: 33
2020-01-09 17:25:06 Failed suite prepare: 1
    - google:ubuntu-core-20-64:tests/regression/
2020-01-09 17:25:06 Failed suite restore: 2
    - google:ubuntu-core-20-64:tests/regression/
    - google:ubuntu-core-20-64:tests/regression/
2020-01-09 17:25:06 Failed project restore: 1
    - google:ubuntu-core-20-64:project
error: unsuccessful run

As such, I restarted it since there wasn't more details in the logs

@pedronis pedronis changed the title boot: split makebootable family into its own file boot: split MakeBootable implementations into their own file Jan 9, 2020
@mvo5 mvo5 merged commit 20dd02b into snapcore:master Jan 10, 2020
@anonymouse64 anonymouse64 deleted the tweak/split-boot-go-make-bootable branch November 25, 2020 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Simple 😃 A small PR which can be reviewed quickly
Projects
None yet
3 participants