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

feat: extend version with more build info #4069

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

JakobLichterfeld
Copy link
Collaborator

closes: #3682

@JakobLichterfeld JakobLichterfeld added the enhancement New feature or request label Jul 9, 2024
Copy link

netlify bot commented Jul 9, 2024

Deploy Preview for teslamate ready!

Name Link
🔨 Latest commit 5b772d1
🔍 Latest deploy log https://app.netlify.com/sites/teslamate/deploys/668e2fbec038b500085de185
😎 Deploy Preview https://deploy-preview-4069--teslamate.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@JakobLichterfeld
Copy link
Collaborator Author

JakobLichterfeld commented Jul 9, 2024

image

Not yet working correctly in our ghcr image, as git command is not available in our docker file

@JakobLichterfeld JakobLichterfeld marked this pull request as draft July 9, 2024 12:30
@brianmay
Copy link
Collaborator

brianmay commented Jul 9, 2024

Not yet working correctly in our ghcr image, as git command is not available in our docker file

I would imagine the nix flake build will have the same issue. The git repo stuff not available in the build.

Think we will need to use environment variables, as per #3682 (comment).

For Docker these need to be passed to the docker build command. --build-arg=arg=value

Nix is easier, with something like this would create the environment variables (this needs to be used in appropriate place):

 build_env = {
          BUILD_DATE =
            with flockenzeit.lib.splitSecondsSinceEpoch { } self.lastModified;
            "${F}T${T}${Z}";
          VCS_REF = "${self.rev or "dirty"}";
        };

Although it is unfortunate you need the flockenzeit just to format the date time. This doesn't use the VERSION file, combining the two shouldn't be hard.

@brianmay
Copy link
Collaborator

brianmay commented Jul 9, 2024

Forgot to say:

  • Version number should be set at build time. Oh wait, I think we already do this. Good.
  • Suggest using environment variables as first preference. If they are not defined, then fall back to something else.

@JakobLichterfeld
Copy link
Collaborator Author

I would imagine the nix flake build will have the same issue. The git repo stuff not available in the build.

Yeah, assume this as well. As we use a two staged docker build, we could add git in the builder step to have the command available with no real downside imo. Will test this. In the end build env are the best choice imo as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend version with more build info
2 participants