-
Notifications
You must be signed in to change notification settings - Fork 286
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
Makefile: simpily ledger build tags #2449
Conversation
|
|
||
# process linker flags | ||
ldflags = -X github.com/cosmos/cosmos-sdk/version.Name=celestia-app \ | ||
-X github.com/cosmos/cosmos-sdk/version.AppName=celestia-appd \ | ||
-X github.com/cosmos/cosmos-sdk/version.Version=$(VERSION) \ | ||
-X github.com/cosmos/cosmos-sdk/version.Commit=$(COMMIT) \ | ||
-X "github.com/cosmos/cosmos-sdk/version.BuildTags=$(build_tags_comma_sep)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this line had zero impact on ledger it seemed. If someone knows why this line is needed, I'm happy to add it back in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears to have been added in #360 but build_tags_comma_sep
wasn't defined in that PR so it seems like it was a no-op when first introduced.
|
||
# process linker flags | ||
ldflags = -X github.com/cosmos/cosmos-sdk/version.Name=celestia-app \ | ||
-X github.com/cosmos/cosmos-sdk/version.AppName=celestia-appd \ | ||
-X github.com/cosmos/cosmos-sdk/version.Version=$(VERSION) \ | ||
-X github.com/cosmos/cosmos-sdk/version.Commit=$(COMMIT) \ | ||
-X "github.com/cosmos/cosmos-sdk/version.BuildTags=$(build_tags_comma_sep)" | ||
ldflags += $(LDFLAGS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LDFLAGS
isn't defined anywhere so this line is a no-op.
Codecov Report
@@ Coverage Diff @@
## main #2449 +/- ##
=======================================
Coverage 20.61% 20.61%
=======================================
Files 131 131
Lines 15270 15270
=======================================
Hits 3148 3148
Misses 11820 11820
Partials 302 302 📢 Have feedback on the report? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clean up!
[nit] PR title could use a conventional commit prefix so that the squashed commit has a conventional commit prefix so that the auto generated release notes are easy to scan and identify if the next release should be a major, minor, or patch release.
|
||
# process linker flags | ||
ldflags = -X github.com/cosmos/cosmos-sdk/version.Name=celestia-app \ | ||
-X github.com/cosmos/cosmos-sdk/version.AppName=celestia-appd \ | ||
-X github.com/cosmos/cosmos-sdk/version.Version=$(VERSION) \ | ||
-X github.com/cosmos/cosmos-sdk/version.Commit=$(COMMIT) \ | ||
-X "github.com/cosmos/cosmos-sdk/version.BuildTags=$(build_tags_comma_sep)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears to have been added in #360 but build_tags_comma_sep
wasn't defined in that PR so it seems like it was a no-op when first introduced.
In the process of working on the `goreleaser` PR I discovered that how we are building the `BUILD_TAGS` is overly verbose and unnecessary. Ultimately we are just trying to add `ledger` to `-tags`. Hardcoding it and adding a note in the README for the 2 distributions that aren't supported seemed easier to maintain. (cherry picked from commit 67241f9) # Conflicts: # specs/src/SUMMARY.md
This is an automatic backport of pull request #2449 done by [Mergify](https://mergify.com). Closes #2597 --------- Co-authored-by: Matthew Sevey <[email protected]> Co-authored-by: Rootul P <[email protected]>
In the process of working on the
goreleaser
PR I discovered that how we are building theBUILD_TAGS
is overly verbose and unnecessary.Ultimately we are just trying to add
ledger
to-tags
. Hardcoding it and adding a note in the README for the 2 distributions that aren't supported seemed easier to maintain.