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

ci: integrate go releaser #3423

Merged
merged 5 commits into from
Oct 31, 2023
Merged

ci: integrate go releaser #3423

merged 5 commits into from
Oct 31, 2023

Conversation

snobbee
Copy link
Contributor

@snobbee snobbee commented Oct 27, 2023

Integrate goreleaser to improve release process.

Copy link
Contributor

@jzvikart jzvikart left a comment

Choose a reason for hiding this comment

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

What exactly does goreleaser do and how does it benefit us?
Our current official way of building is "make install". This should always produce the official sifnoded binary. We should only have one way of building, and it should run locally in exactly the same way as in GitHub actions.

We already have too many different ways to do the same thing, so before we introduce any new tool like this it's important to make sure we don't introduce even more ways, but streamline the process and clean up everything except "the single source of truth".

@snobbee
Copy link
Contributor Author

snobbee commented Oct 28, 2023

What exactly does goreleaser do and how does it benefit us? Our current official way of building is "make install". This should always produce the official sifnoded binary. We should only have one way of building, and it should run locally in exactly the same way as in GitHub actions.

We already have too many different ways to do the same thing, so before we introduce any new tool like this it's important to make sure we don't introduce even more ways, but streamline the process and clean up everything except "the single source of truth".

I understand the importance of maintaining a single, consistent build process. The intention behind proposing goreleaser is not to introduce another method of building, but to enhance our current one. Here's why:

  1. Cross-Platform Binaries: While "make install" works well, it may not be the most efficient way to produce binaries for all major platforms. goreleaser excels in this regard, enabling us to easily generate binaries for multiple platforms without the complexity that might come with a Makefile alone.

  2. Adoption in the Cosmos Ecosystem: Several projects within the Cosmos ecosystem have already adopted goreleaser. For instance, both Osmosis and Cosmos SDK use it. This not only showcases its reliability but also hints at the potential benefits it offers, especially in terms of standardizing builds across the ecosystem.

  3. Streamlined Release Process: Take a look at the Osmo release to see the output goreleaser can produce. It not only builds the binaries but also structures them in a clean, organized manner, which can be extremely beneficial for our users and for ensuring clarity in our release process.

Integrating goreleaser doesn't mean we're introducing a new build method. Instead, we're enhancing our existing one, ensuring that it remains the single source of truth, but is more efficient, consistent, and in line with what's becoming a standard in the Cosmos ecosystem.

@jzvikart
Copy link
Contributor

@snobbee That's a great sales pitch, but it doesn't really answer the question of how it benefits us. For example, due to Peggy our build process has lot of dependencies and complications, including npm, docker, smart contracts, yarn, protobuf etc., They are highly complex and they break all the time, causing us a lot of grief. The real source of our pain is technical debt and goreleaser will most definitely not help us with that. The proper thing to do would be to focus on redesigning our build process from the ground up, in a way that covers all our needs holistically, succintly and comprehensively. It will take a lot of effort to pay off the debt, and until then, any partial solution will have a very limited benefits (if at all). I'm not opposing using goreleaser, my concern is that the longer we wait the higher the price we are paying.

pgoos
pgoos previously approved these changes Oct 28, 2023
Copy link
Contributor

@pgoos pgoos left a comment

Choose a reason for hiding this comment

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

LGTM

@pgoos
Copy link
Contributor

pgoos commented Oct 28, 2023

LGTM

@snobbee That's a great sales pitch, but it doesn't really answer the question of how it benefits us. For example, due to Peggy our build process has lot of dependencies and complications, including npm, docker, smart contracts, yarn, protobuf etc., They are highly complex and they break all the time, causing us a lot of grief. The real source of our pain is technical debt and goreleaser will most definitely not help us with that. The proper thing to do would be to focus on redesigning our build process from the ground up, in a way that covers all our needs holistically, succintly and comprehensively. It will take a lot of effort to pay off the debt, and until then, any partial solution will have a very limited benefits (if at all). I'm not opposing using goreleaser, my concern is that the longer we wait the higher the price we are paying.

We don't even know if we're coming back to peggy so maybe let's postpone any efforts fixing the peggy build process. @jzvikart what do you think?

@jzvikart
Copy link
Contributor

@pgoos Absolutely, if we're not going back to Peggy, dropping support for it would be a massive relief. But until we get a decision to permanently drop Peggy, we have to keep supporting it. Let's talk about this during the next meeting.

@jzvikart
Copy link
Contributor

jzvikart commented Oct 29, 2023

@snobbee If we're going to uses this, please also update make install target so that it uses goreleaser and keeps builds consistent across different environments rather than just adding another target.

@codecov
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #3423 (0406ed7) into master (0c63583) will increase coverage by 0.00%.
Report is 5 commits behind head on master.
The diff coverage is 30.55%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3423   +/-   ##
=======================================
  Coverage   43.35%   43.36%           
=======================================
  Files         166      166           
  Lines       15895    15904    +9     
=======================================
+ Hits         6891     6896    +5     
- Misses       8591     8595    +4     
  Partials      413      413           
Files Coverage Δ
app/app.go 90.20% <100.00%> (+0.10%) ⬆️
cmd/sifnoded/cmd/ibc-diag.go 15.74% <ø> (ø)
x/dispensation/types/types.go 100.00% <100.00%> (ø)
x/ibctransfer/helpers/conversion_helper.go 37.19% <ø> (ø)
x/ibctransfer/keeper/msg_server.go 86.95% <ø> (ø)
x/ibctransfer/types/mocks/expected_keepers_gen.go 25.33% <ø> (ø)
app/setup_handlers.go 10.52% <0.00%> (+1.83%) ⬆️
x/ibctransfer/onrecv_packet.go 0.00% <0.00%> (ø)
x/tokenregistry/types/mock/keeper_gen.go 0.00% <0.00%> (ø)

@snobbee snobbee enabled auto-merge (squash) October 31, 2023 05:17
Copy link
Contributor

@pgoos pgoos left a comment

Choose a reason for hiding this comment

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

LGTM

@snobbee snobbee merged commit 8026528 into master Oct 31, 2023
7 checks passed
@snobbee snobbee deleted the ci/goreleaser branch October 31, 2023 10:38
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