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

Put aktualizr-cert-provider in garage_deploy.deb #1218

Merged
merged 3 commits into from
May 31, 2019

Conversation

lbonn
Copy link
Contributor

@lbonn lbonn commented May 24, 2019

For convenience

@zabbal
Copy link
Contributor

zabbal commented May 28, 2019

Seems reasonable at a quick glance, provided you can fix the CI.

CMakeLists.txt Outdated
@@ -334,7 +334,7 @@ add_library(jsoncpp OBJECT third_party/jsoncpp/jsoncpp.cpp)

# General packaging configuration
set(CPACK_GENERATOR "DEB")
set(CPACK_DEBIAN_PACKAGE_MAINTAINER "anton.gerasimov@here.com")
set(CPACK_DEBIAN_PACKAGE_MAINTAINER "patrick.vacek@here.com")
Copy link
Collaborator

Choose a reason for hiding this comment

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

lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe I should have asked you before indeed, sorry

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's fine! I was just amused.

@@ -8,9 +8,11 @@ set(HEADERS packagemanagerconfig.h
packagemanagerfake.h
packagemanagerinterface.h)

add_library(package_manager OBJECT ${SOURCES})
add_library(package_manager OBJECT packagemanagerfactory.cc packagemanagerfake.cc packagemanagerinterface.cc)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a bit less than ideal; should we instead remove packagemanagerconfig.cc from SOURCES?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok changed

@lbonn
Copy link
Contributor Author

lbonn commented May 28, 2019

Hm yes I really don't know what's going on with CI, that's quite annoying.

@lbonn lbonn force-pushed the feat/akt-cert-provider-pkg branch from dde9584 to 5cc52d1 Compare May 28, 2019 13:08
@codecov-io
Copy link

codecov-io commented May 28, 2019

Codecov Report

Merging #1218 into master will increase coverage by 0.15%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1218      +/-   ##
==========================================
+ Coverage   79.35%   79.51%   +0.15%     
==========================================
  Files         168      170       +2     
  Lines       10029    10028       -1     
==========================================
+ Hits         7959     7974      +15     
+ Misses       2070     2054      -16
Impacted Files Coverage Δ
...ibaktualizr/package_manager/packagemanagerconfig.h 91.66% <ø> (ø) ⬆️
...c/aktualizr_secondary/aktualizr_secondary_config.h 100% <ø> (ø) ⬆️
src/libaktualizr/bootloader/bootloader.cc 50% <ø> (-12.23%) ⬇️
src/aktualizr_info/aktualizr_info_config.h 100% <ø> (ø) ⬆️
src/libaktualizr/storage/invstorage.cc 88.51% <ø> (-1.7%) ⬇️
src/libaktualizr/storage/storage_config.h 88.88% <ø> (ø) ⬆️
src/libaktualizr/bootloader/bootloader_config.h 100% <ø> (ø) ⬆️
...baktualizr/package_manager/packagemanagerconfig.cc 95.83% <100%> (-0.17%) ⬇️
src/libaktualizr/bootloader/bootloader_config.cc 89.28% <89.28%> (ø)
src/libaktualizr/storage/storage_config.cc 95.65% <95.65%> (ø)
... and 5 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 66c57c2...663f655. Read the comment docs.

lbonn added 3 commits May 28, 2019 17:12
This way, we avoid having to pull every full library when linking with config.

Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn lbonn force-pushed the feat/akt-cert-provider-pkg branch from 3314db3 to 663f655 Compare May 28, 2019 15:13
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.

Seems fine once CI is green. I think there is some risk of confusion here, but I'm willing to trust your vision, and it does solve the immediate problem with cert-provider.

@pattivacek pattivacek merged commit 7bde02e into master May 31, 2019
@pattivacek pattivacek deleted the feat/akt-cert-provider-pkg branch May 31, 2019 13:20
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.

None yet

4 participants