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

fix: price-feeder version #1291

Merged
merged 11 commits into from
Aug 30, 2022
Merged

fix: price-feeder version #1291

merged 11 commits into from
Aug 30, 2022

Conversation

rbajollari
Copy link
Contributor

@rbajollari rbajollari commented Aug 26, 2022

…on command

Description

closes: #1281

This bug is happening because the price-feeder version command is trying to open the go.mod file of the price-feeder but if you're outside of that directory it doesn't know where that go.mod file is. My proposed solution is to add an argument to the version command for the directory of the price-feeder in your environment. Let me know if you have a better way to approach this.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • added appropriate labels to the PR
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@RafilxTenfen
Copy link
Contributor

Hey @rbajollari, So the idea is that the price-feeder version doesn't need any go.mod to display it's current version
instead when we are building the binary with make build, it should somewhat be able to save the current git commit / tag version in which the binary was build

An idea how to do that it to look how umeed is doing umeed version

Copy link
Member

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

Please use the pattern of setting a version during the build time:

You need to adopt it in price-feeder by adding Version variable to price-feeder/main.go and setting it through ldflags as outlined above.

@rbajollari rbajollari changed the title Bug: price-feeder version Fix: price-feeder version Aug 26, 2022
@rbajollari rbajollari changed the title Fix: price-feeder version fix: price-feeder version Aug 26, 2022
Copy link
Collaborator

@adamewozniak adamewozniak left a comment

Choose a reason for hiding this comment

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

Nice! Looks good to me!

@adamewozniak
Copy link
Collaborator

@robert-zaremba please check this out when you can

Copy link
Contributor

@RafilxTenfen RafilxTenfen left a comment

Choose a reason for hiding this comment

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

Tested ACK, just a question on umeed we have umeed version which just returns like main-1f36d964a9449cb307806bef68eb90197b845274 and the --long flag, which it prints all the build deps, build flags used and cosmos SDK version

What do you think about following that pattern?

Copy link
Member

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

utACK

@adamewozniak
Copy link
Collaborator

adamewozniak commented Aug 30, 2022

Tested ACK, just a question on umeed we have umeed version which just returns like main-1f36d964a9449cb307806bef68eb90197b845274 and the --long flag, which it prints all the build deps, build flags used and cosmos SDK version

What do you think about following that pattern?

Hmm, we could add a task to add the "--long" flag, but atm I think the contents of this PR are enough (we mostly care about commit # and cosmos version with this tool) - wdyt @RafilxTenfen / @rbajollari ?

@RafilxTenfen
Copy link
Contributor

Tested ACK, just a question on umeed we have umeed version which just returns like main-1f36d964a9449cb307806bef68eb90197b845274 and the --long flag, which it prints all the build deps, build flags used and cosmos SDK version
What do you think about following that pattern?

Hmm, we could add a task to add the "--long" flag, but atm I think the contents of this PR are enough (we mostly care about commit # and cosmos version with this tool) - wdyt @RafilxTenfen / @rbajollari ?

Agree

@mergify mergify bot merged commit 47b4415 into main Aug 30, 2022
@mergify mergify bot deleted the ryan/price-feeder-version branch August 30, 2022 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

price-feeder version
4 participants