Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

OLPSUP-8795: Import the initial hash from file for IP Secondary #1485

Merged
merged 2 commits into from
Dec 12, 2019

Conversation

mike-sul
Copy link
Collaborator

Signed-off-by: Mykhaylo Sul [email protected]

@lbonn
Copy link
Contributor

lbonn commented Dec 11, 2019

Nice. Do you think adding a non-regression test would be possible here?

@mike-sul
Copy link
Collaborator Author

Nice. Do you think adding a non-regression test would be possible here?

Everything is possible :). The question is if it is reasonable. I am just not sure if the current approach to setting of the initial version is the right one. If not, then we have to rework it, so adding test to something that we are going to throw away does not make sense, otherwise I can add some test. Let's discuss it tomorrow during the meeting.

pattivacek
pattivacek previously approved these changes Dec 12, 2019
@pattivacek pattivacek dismissed their stale review December 12, 2019 09:24

Please update the changelog, otherwise looks good!

@pattivacek
Copy link
Collaborator

To summarize discussion: testing is of course a great idea, but since we think we want to redo this logic really soon anyway, perhaps a test for this quick-fix logic is not such a great use of our time.

… file prepared by the bitbake process

Signed-off-by: Mykhaylo Sul <[email protected]>
@mike-sul mike-sul force-pushed the fix/OLPSUP-8795/incorrect-hash-reported branch from c380922 to 4cef29e Compare December 12, 2019 09:46
@codecov-io
Copy link

codecov-io commented Dec 12, 2019

Codecov Report

Merging #1485 into master will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1485      +/-   ##
==========================================
+ Coverage   80.66%   80.79%   +0.12%     
==========================================
  Files         184      184              
  Lines       11138    11141       +3     
==========================================
+ Hits         8985     9001      +16     
+ Misses       2153     2140      -13
Impacted Files Coverage Δ
...c/aktualizr_secondary/aktualizr_secondary_config.h 100% <ø> (ø) ⬆️
src/libaktualizr/storage/invstorage.h 97.56% <ø> (ø) ⬆️
.../aktualizr_secondary/aktualizr_secondary_config.cc 94.8% <100%> (+0.13%) ⬆️
.../aktualizr_secondary/aktualizr_secondary_common.cc 88.57% <100%> (+0.33%) ⬆️
src/libaktualizr/primary/initializer.cc 80.64% <0%> (-1.3%) ⬇️
src/aktualizr_primary/main.cc 80.37% <0%> (-0.94%) ⬇️
src/libaktualizr/primary/sotauptaneclient.cc 88.71% <0%> (-0.15%) ⬇️
src/libaktualizr/storage/sqlstorage.cc 77.45% <0%> (+0.66%) ⬆️
src/libaktualizr/storage/sqlstorage_base.cc 79.72% <0%> (+5.4%) ⬆️
src/libaktualizr/storage/sqlstorage_base.h 100% <0%> (+40%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a68cd5...4cef29e. Read the comment docs.

@mike-sul mike-sul requested a review from lbonn December 12, 2019 09:50
@mike-sul
Copy link
Collaborator Author

@lbonn @patrickvacek please, check out if I updated the changelog correctly.

Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

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

Perfect, thanks!

@pattivacek pattivacek merged commit 6633d0d into master Dec 12, 2019
@pattivacek pattivacek deleted the fix/OLPSUP-8795/incorrect-hash-reported branch December 12, 2019 11:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants