adding receipt package from repository-lib#127
adding receipt package from repository-lib#127Pedram-A-Keyvani wants to merge 4 commits intomainfrom
Conversation
kdp-cloud
left a comment
kdp-cloud
left a comment
There was a problem hiding this comment.
As far as I can assess the changes, LGTM.
There are some valid concerns expressed in the Dockerfiles regarding the injection of secrets through ENV or ARG vars. It would be good to investigate the mounting of secrets at build time: https://docs.docker.com/build/building/secrets/.
I think it's a good idea to have @dipayan1985 review this PR as well, as he is the one that will be consuming the library.
Nice work though! Thanks!
|
I changed the "build args" to the "secrets" approach.
|
kdp-cloud
requested review from
apriltuesday and
dipayan1985
and removed request for
bedroesb
kdp-cloud
left a comment
kdp-cloud
left a comment
There was a problem hiding this comment.
Thanks @Pedram-A-Keyvani!
I approve you PR but would like either @dipayan1985 or @apriltuesday to review it as well before merging.
apriltuesday
left a comment
apriltuesday
left a comment
There was a problem hiding this comment.
Looks good to me as well, I believe the current plan is to merge #123 first and then resolve conflicts in this one (correct me if I'm wrong Pedram), so I'll save my ✅ till that's done.
3 participants
No description provided.