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

build(dockerfile): ensure that docker in installed so that commitsha is read during build #1333

Merged

Conversation

machi1990
Copy link
Contributor

@machi1990 machi1990 commented Sep 30, 2022

This follows up on #1321, notably #1321 (comment)

Description

Verification Steps

Checklist (Definition of Done)

  • All acceptance criteria specified in JIRA have been completed
  • Unit and integration tests added that prove the fix is effective or the feature works (tested against emulated and non-emulated OCM environment)
  • Documentation added for the feature
  • CI and all relevant tests are passing
  • Code Review completed
  • Verified independently by reviewer
  • All PR comments are resolved either by addressing them or creating follow up tasks
  • Required metrics/dashboards/alerts have been added (or PR created).
  • Required Standard Operating Procedure (SOP) is added.
  • JIRA has been created for changes required on the client side

@machi1990 machi1990 requested a review from a team as a code owner September 30, 2022 15:48
@github-actions github-actions bot added kafka service-template Label categorizing all the service templates changes labels Sep 30, 2022
@machi1990
Copy link
Contributor Author

/cc @valeriiashapoval

@miguelsorianod
Copy link
Contributor

Do we need that in any other image/environment? @machi1990

@miguelsorianod
Copy link
Contributor

miguelsorianod commented Sep 30, 2022

This is very surprising behavior as all I found in the Go documentation is:

Version control information is embedded if the go command is invoked in a directory within a Git, Mercurial, Fossil, or Bazaar repository, and the main package and its containing main module are in the same repository. 

Which does not mention the need of having git installed

@codecov
Copy link

codecov bot commented Sep 30, 2022

Codecov Report

Merging #1333 (a86628a) into main (3a5738c) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1333   +/-   ##
=======================================
  Coverage   83.66%   83.66%           
=======================================
  Files         139      139           
  Lines       12436    12436           
=======================================
  Hits        10404    10404           
  Misses       1666     1666           
  Partials      366      366           
Flag Coverage Δ
unittests 83.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@machi1990
Copy link
Contributor Author

This is very surprising behavior as all I found in the Go documentation is:

Version control information is embedded if the go command is invoked in a directory within a Git, Mercurial, Fossil, or Bazaar repository, and the main package and its containing main module are in the same repository. 

Which does not mention the need of having git installed

Yes, I think the documentation could have been better on that regard.
Now everything relies on the assumption that users can make when reading .."Version control information is embedded if the go command is invoked in a directory within a Git, Mercurial, Fossil, or Bazaar repository" that if you are in a vcs repo, then you must have a corresponding vcs tool installed.

The requirement of having git initially caused some issues e.g golang/go#51723

@machi1990 machi1990 merged commit 83a7033 into bf2fc6cc711aee1a0c2a:main Sep 30, 2022
@machi1990 machi1990 deleted the build/add-git-docker-build branch September 30, 2022 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kafka service-template Label categorizing all the service templates changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants