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

Setup COMMIT_SHA and COMMIT_DATE build args in the Docker image #2550

Closed
wants to merge 0 commits into from

Conversation

brunoocasali
Copy link
Member

@brunoocasali brunoocasali commented Jun 24, 2022

Summary of the changes:

  • Rename from check-tag-format to build-metadata because now it does more than just check tag format (I was not sure about creating one step for just assigning a variable).
  • Add the build-args definition from https://github.com/docker/build-push-action#inputs which is supposed to work exactly as docker build --build-arg.
  • I'm not sure about the usage of git on line 34 because I don't know if it will be available at that point.

Fixes #2028

@curquiza
Copy link
Member

curquiza commented Jun 27, 2022

Hello @brunoocasali!
Thanks for your PR!!! 🙌

Since we have changed some parts of this CI for v0.28.0 (that are currently in release-v0.28.0 and not on main) I will not merge your PR until v0.28.0 is out. You will probably need to rebase, I will guide you through the git conflicts if necessary.

Once it's rebased, I will be able to review it, currently, it's not because I have two contexts to consider when reviewing 😅

Thanks again!!

@curquiza curquiza self-requested a review June 27, 2022 08:54
@curquiza curquiza added this to the v0.29.0 milestone Jun 27, 2022
@brunoocasali
Copy link
Member Author

No problem @curquiza let me know when you're ready :D

@curquiza curquiza added maintenance Issue about maintenance (CI, tests, refacto...) github actions Pull requests that update GitHub Actions code labels Jul 5, 2022
@curquiza
Copy link
Member

Hello @brunoocasali!
You can now rebase your branch when you have the time 🎉 As I expected, you have to fix some git conflict, sorry for that... 😅
If you don't know how to solve them, we can do this together during one of our meetings!

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Thanks @brunoocasali for the PR

First I would keep 2 separated jobs

  • let the Check tag format -> this should only run for github.event_name != 'schedule'
  • another new one, your Build git metadata -> this should run without any condition

Secondly, when I tried to run the CI by pushing a random tag (test-git-tag) I got an error:
https://github.com/meilisearch/meilisearch/runs/7388047405?check_suite_focus=true

@brunoocasali
Copy link
Member Author

GitHub auto-closed my PR when I synced changes with my remote 🤷‍♂️

@curquiza curquiza removed this from the v1.0.0 milestone Dec 7, 2022
bors bot added a commit that referenced this pull request Dec 8, 2022
3212: Setup COMMIT_SHA and COMMIT_DATE build args in the Docker image r=curquiza a=brunoocasali

GitHub auto-closed my PR when I synced changes with my remote 🤷‍♂️  #2550
The last PR #3205 were closed to help `@curquiza` test the CI.

In any case, the summary of changes is quite similar:

- Fix `git` usage from my last attempt (when you use `actions/checkout`) you get the `git` command to use.
- Add the `build-args` definition from https://github.com/docker/build-push-action#inputs, which is supposed to work precisely as docker build `--build-arg`. 

Fixes #2028

The result will be like this:

<img width="556" alt="image" src="https://user-images.githubusercontent.com/4116980/206019608-2713559a-1f58-4ff3-9fec-7720783993ac.png">

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github actions Pull requests that update GitHub Actions code maintenance Issue about maintenance (CI, tests, refacto...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker CI: should not display unknown as commit SHA and commit date
2 participants