Skip to content

Conversation

@mtojek
Copy link
Contributor

@mtojek mtojek commented Jul 16, 2021

This PR fixes missing build info while user runs elastic-package version.

Issue: #32

Tested:

$ goreleaser build --skip-validate --single-target
$ ./dist/elastic-package_darwin_amd64/elastic-package version
elastic-package has been installed.
elastic-package version-hash e59a09c (build time: 2021-07-16T13:11:19+02:00)

@jsoriano Thank you for suggestions in #431 (comment) . I suggest to review and merge this PR, and then we can review the entire working solution in terms of further adjustments. This distribution method is not advertised yet, so we can disable it if we decide it doesn't meet our requirements.

@mtojek mtojek self-assigned this Jul 16, 2021
@mtojek mtojek requested a review from jsoriano July 16, 2021 11:30
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Nice to see how goreleaser can be customized.

I think we should add the version itself to the output of elastic-package version.

builds:
- ldflags:
- -X github.com/elastic/elastic-package/internal/version.CommitHash={{.ShortCommit}}
- -X github.com/elastic/elastic-package/internal/version.BuildTime={{.Timestamp}} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Users are going to be probably more interested about the version itself 🙂 Add the tag to the output of elastic-package version too, so it displays something like this?

$ goreleaser build --skip-validate --single-target
$ ./dist/elastic-package_darwin_amd64/elastic-package version
elastic-package has been installed.
elastic-package v0.9.1 version-hash e59a09c (build time: 2021-07-16T13:11:19+02:00)
Suggested change
- -X github.com/elastic/elastic-package/internal/version.BuildTime={{.Timestamp}}
- -X github.com/elastic/elastic-package/internal/version.BuildTime={{.Timestamp}}
- -X github.com/elastic/elastic-package/internal/version.Tag={{.Tag}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to do this in the next follow-up (just after this one :) ) as it requires a bit of coding. With this PR I want to fix first what's still broken.

If you prefer I can try to modify version's logic in this PR (both options work for me).

Copy link
Member

Choose a reason for hiding this comment

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

SGTM to do it in a follow up.

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-07-16T11:30:32.097+0000

  • Duration: 24 min 40 sec

  • Commit: 832d1c3

Test stats 🧪

Test Results
Failed 0
Passed 316
Skipped 4
Total 320

Trends 🧪

Image of Build Times

Image of Tests

@mtojek mtojek merged commit 46873e1 into elastic:master Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants