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

Goreleaser #180

Merged
merged 9 commits into from
Jul 31, 2023
Merged

Goreleaser #180

merged 9 commits into from
Jul 31, 2023

Conversation

spinillos
Copy link
Member

@spinillos spinillos commented Jul 4, 2023

Adds Goreleaser to generate semantic versioning to thema.

Contributes to: #172

@spinillos spinillos requested a review from joanlopez July 4, 2023 11:32
archives:
- format: tar.gz
format_overrides:
- goos: windows
Copy link
Contributor

Choose a reason for hiding this comment

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

For Windows, we'll also probably need to add .exe extension to the binary file.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't add anything because I wasn't really sure. This is from the basic template.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, np! I experienced that in a different project, that's why I'm suggesting it hahaha, but my first experience was exactly as yours (following the basic template), so don't worry at all!

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm seeing into goreleaser documentation that available formats are the following:

Valid options are `tar.gz`, `tgz`, `tar.xz`, `txz`, tar`, `gz`, `zip` and `binary`.

So I understand that could we use binary?

Copy link
Contributor

@joanlopez joanlopez Jul 10, 2023

Choose a reason for hiding this comment

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

I think the current format (zip) is fine, because it will generate a compressed file with the resulting binary inside (similar to other platforms, just replacing tar by zip, which is way more common on Windows) - that's ok!

What I suggested at the beginning is changing the name of the resulting binary (what will end up within that zip file), from just thema (or whatever it is now) to thema.exe, because I think that, based on previous experiences, on Windows (by default), a file needs to have the .exe extension to be considered an executable 😅

Hope it makes more sense now! 🙏🏻

Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@joanlopez joanlopez left a comment

Choose a reason for hiding this comment

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

Thanks so much @spinillos!
Looks like a good first iteration, I've just left few comments I think we could iterate a bit before shipping it!

@sdboyer
Copy link
Contributor

sdboyer commented Jul 31, 2023

thanks!

@sdboyer sdboyer merged commit 5a1a452 into main Jul 31, 2023
@sdboyer
Copy link
Contributor

sdboyer commented Jul 31, 2023

Hmm, there seems to be an issue with the tag regex: https://github.com/grafana/thema/actions/runs/5713950449. Annoying that the actions engine only complained after merging...

But, it's not immediately obvious to me why that's a problem, as the regex itself is valid. @spinillos could you investigate and fix?

@joanlopez
Copy link
Contributor

joanlopez commented Jul 31, 2023

Hmm, there seems to be an issue with the tag regex: https://github.com/grafana/thema/actions/runs/5713950449. Annoying that the actions engine only complained after merging...

But, it's not immediately obvious to me why that's a problem, as the regex itself is valid. @spinillos could you investigate and fix?

Seems related with: https://github.com/orgs/community/discussions/55854
TL;DR: Looks like it has no full regex capabilities.

So, I guess we'll need to either simplify it or find an alternative.
After a very quick search, I've found a Parse Semver action, which I guess could help to define a conditional workflow.

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.

4 participants