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

feat: Add ability to check DefraDB CLI version. #339

Merged
merged 5 commits into from
Apr 20, 2022

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Apr 12, 2022

Adds the ability to check DefraDB's latest release version.
Resolves #259

Usage:
  defradb version [flags]

Flags:
  -f, --format string   The version's format can be one of: 'short', 'json'

Example [default]:

>> $ defradb version

image

Example [short flag]:

>> $ defradb version -f short

0.2.1

Example [json flag]:

>> $ defradb version --format json

{"tag":"0.2.1","commit":"e4328e0","date":"2022-03-07T00:12:07Z"}

```
Usage:
  defradb version [flags]

Flags:
      --f string   The version's format can be one of: 'short', 'json'
```

Example [default]:
>> `defradb version`
```
DefraDB's Version Information:
  *  🏷  version tag  = 0.2.1
  *   # build commit = e4328e0
  *  📅 release date = 2022-03-07T00:12:07Z
```

Example [short flag]:
>> `defradb version --f short`
```
0.2.1
```

Example [json flag]:
>> `defradb version --f json`
```{"tag":"0.2.1","commit":"e4328e0","date":"2022-03-07T00:12:07Z"}```
@shahzadlone shahzadlone added feature New feature or request action/no-benchmark Skips the action that runs the benchmark. labels Apr 12, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.3 milestone Apr 12, 2022
@shahzadlone shahzadlone self-assigned this Apr 12, 2022
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

Looks really nice :) I do question slightly as to whether it is worth an extra 3rd party dependency though - but will trust your decision on that

@shahzadlone
Copy link
Member Author

Looks really nice :) I do question slightly as to whether it is worth an extra 3rd party dependency though - but will trust your decision on that

Are you referring to color support (fatih/color)?

@shahzadlone
Copy link
Member Author

There is actually one thing we have to have a discussion on. The commit hash that is returned for the version probably shouldn't be hardcoded. Because the releasing tag commit (final commit) will not know it's own hash in advance to place in the commit (as commit hashes dependent on the time the commit happened at and who made the commit). The other approach might be to put the hash of one commit before the released tag.

@AndrewSisley
Copy link
Contributor

Looks really nice :) I do question slightly as to whether it is worth an extra 3rd party dependency though - but will trust your decision on that

Are you referring to color support (fatih/color)?

Yes

Copy link
Contributor

@orpheuslummis orpheuslummis left a comment

Choose a reason for hiding this comment

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

LGTM :)

I want to add that I'll provide an update to the implementation as part the CLI overhaul I'm working on to include things like

  • top-level version package to have default version information of potentially multiple components
  • Makefile -> git describe -> build tags defining defradb version variable
  • as part of the configuration overhaul i'm introducing a --no-color which will conditionally turn off the coloring, and an output global flag (text, json) to globally decide on the default output type used

I think the added color dependency looks fine - convenient, popular/supported, probably not heavy

@shahzadlone shahzadlone mentioned this pull request Apr 18, 2022
cli/defradb/cmd/version.go Outdated Show resolved Hide resolved
@jsimnz
Copy link
Member

jsimnz commented Apr 18, 2022

Is this worth pursuing if @orpheuslummis plans on re-implementing it as the CLI overhaul.

My approach for this would be a dedicated version package, whose vars can be set at build time using LDFlags, like so:

https://github.com/tendermint/tendermint/blob/851c0dc4f3088723c724cb01c4877ee78c3d6ffc/Makefile#L14

and

https://github.com/tendermint/tendermint/blob/851c0dc4f3088723c724cb01c4877ee78c3d6ffc/version/version.go#L6

@shahzadlone
Copy link
Member Author

shahzadlone commented Apr 19, 2022

Is this worth pursuing if @orpheuslummis plans on re-implementing it as the CLI overhaul.
My approach for this would be a dedicated version package, whose vars can be set at build time using LDFlags, like so:
https://github.com/tendermint/tendermint/blob/851c0dc4f3088723c724cb01c4877ee78c3d6ffc/Makefile#L14
and
https://github.com/tendermint/tendermint/blob/851c0dc4f3088723c724cb01c4877ee78c3d6ffc/version/version.go#L6

I have added these to #349
Inaddition to this approach I have added another approach in #349 which golangci-lint uses they also use ld build flags however through goreleaser which I think will be a good approach once we have goreleaser setup.

For now as we already have users that are starting to use Defra so I think that merging this is fine as it offers them (those building from source) a quick way to see they are indeed on v0.2.1. This should be a fairly easy refactor / cleanup todo for @orpheuslummis when they decide on goreleaser or tendermint like approach using ld flags. I also see the CLI overhaul also has 10 total subtasks so until the version stuff gets implemented this can still be usable. The LDFlags approach from golangci-lint can actually build on top of this code quite nicely. For completeness here is the future approach from golangci-lint which uses LDFlags through goreleaser (also posted these in #349):

 ldflags: -s -w -X main.version={{.Version}} -X main.commit={{.ShortCommit}} -X main.date={{.Date}}
var (
	// Populated by goreleaser during build
	version = "master"
	commit  = "?"
	date    = ""
)

Unless there is strong opposition to merging this in, I would say it's okay to merge it for now gives the feature quickly for people using and can be built on top of (depending on which ld flag approach is picked)

@shahzadlone shahzadlone requested a review from jsimnz April 20, 2022 01:57
@jsimnz
Copy link
Member

jsimnz commented Apr 20, 2022

Is this worth pursuing if @orpheuslummis plans on re-implementing it as the CLI overhaul.
My approach for this would be a dedicated version package, whose vars can be set at build time using LDFlags, like so:
https://github.com/tendermint/tendermint/blob/851c0dc4f3088723c724cb01c4877ee78c3d6ffc/Makefile#L14
and
https://github.com/tendermint/tendermint/blob/851c0dc4f3088723c724cb01c4877ee78c3d6ffc/version/version.go#L6

I have added these to #349 Inaddition to this approach I have added another approach in #349 which golangci-lint uses they also use ld build flags however through goreleaser which I think will be a good approach once we have goreleaser setup.

For now as we already have users that are starting to use Defra so I think that merging this is fine as it offers them (those building from source) a quick way to see they are indeed on v0.2.1. This should be a fairly easy refactor / cleanup todo for @orpheuslummis when they decide on goreleaser or tendermint like approach using ld flags. I also see the CLI overhaul also has 10 total subtasks so until the version stuff gets implemented this can still be usable. The LDFlags approach from golangci-lint can actually build on top of this code quite nicely. For completeness here is the future approach from golangci-lint which uses LDFlags through goreleaser (also posted these in #349):

 ldflags: -s -w -X main.version={{.Version}} -X main.commit={{.ShortCommit}} -X main.date={{.Date}}
var (
	// Populated by goreleaser during build
	version = "master"
	commit  = "?"
	date    = ""
)

Unless there is strong opposition to merging this in, I would say it's okay to merge it for now gives the feature quickly for people using and can be built on top of (depending on which ld flag approach is picked)

Agree perfectly with this assessment! Lets merge this now as a short term and rework as we need/arrive with the CLI refactor.

Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

LGTM

@shahzadlone shahzadlone merged commit 6a8f990 into develop Apr 20, 2022
@shahzadlone shahzadlone deleted the lone/feat/i-259/cli-version-command branch April 20, 2022 13:35
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
Adds the ability to check DefraDB's latest release version.
Resolves sourcenetwork#259 

```
Usage:
  defradb version [flags]

Flags:
  -f, --format string   The version's format can be one of: 'short', 'json'
```

Example [default]:
>> `defradb version`
```
DefraDB's Version Information:
  *   version tag  = 0.2.1
  *   build commit = e4328e0
  *   release date = 2022-03-07T00:12:07Z
```

Example [short flag]:
```
>> $ defradb version -f short

0.2.1
```

Example [json flag]:
```
>> $ defradb version --format json

{"tag":"0.2.1","commit":"e4328e0","date":"2022-03-07T00:12:07Z"}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version CLI
4 participants