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.

Sign up for GitHub

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

sso-{auth,proxy}: adding version flag #139

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

sso-{auth,proxy}: adding version flag #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
version := "v1.1.0"

commit := $(shell git rev-parse --short HEAD)
version := $(shell go version)
ldflags := '-X "main.goVersion=$(version)"'
Copy link
Contributor

Choose a reason for hiding this comment

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

Go has a runtime.Version() func in the standard library that we can use instead of passing a variable externally into the build process.


build: dist/sso-auth dist/sso-proxy

dist/sso-auth:
mkdir -p dist
go generate ./...
go build -o dist/sso-auth ./cmd/sso-auth
go build -ldflags $(ldflags) -o dist/sso-auth ./cmd/sso-auth

dist/sso-proxy:
mkdir -p dist
go generate ./...
go build -o dist/sso-proxy ./cmd/sso-proxy
go build -ldflags $(ldflags) -o dist/sso-proxy ./cmd/sso-proxy

test:
./scripts/test
Expand Down
8 changes: 8 additions & 0 deletions cmd/sso-auth/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,19 @@ import (
"github.com/kelseyhightower/envconfig"
)

var goVersion string

func init() {
log.SetServiceName("sso-authenticator")
}

func main() {
if len(os.Args) > 1 && os.Args[1] == "version" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to see this as an actual command line flag (i.e. sso-auth --version), rather than a manually-parsed argument. Primarily, my thinking is that it if we implement it that way, it'll automatically show up in sso-auth --help output, so users will know it's an option.

fmt.Println(auth.VERSION)
fmt.Println(goVersion)
Copy link
Contributor

Choose a reason for hiding this comment

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

Aesthetic and nit-picky, but my preference for this would be:

sso-{proxy,auth} vX.XX (built w/goX.XX)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just like to reiterate my request in #36 (comment) that the app version and go version be split across two lines for easier tooling.

In my (fairly limited) experience, the thing that matters when building automation is the app version, and it's a bit annoying to have to do extra work to split it out from the go version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, sorry I missed that in the original issue.

I get it, and can probably get over my distaste. I feel like you would still have to parse it, e.g. by grabbing just the first line of output, but yea, that is easier.

os.Exit(1)
}

logger := log.NewLogEntry()

opts := auth.NewOptions()
Expand Down
8 changes: 8 additions & 0 deletions cmd/sso-proxy/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,19 @@ import (
"github.com/kelseyhightower/envconfig"
)

var goVersion string

func init() {
log.SetServiceName("sso-proxy")
}

func main() {
if len(os.Args) > 1 && os.Args[1] == "version" {
fmt.Println(proxy.VERSION)
fmt.Println(goVersion)
os.Exit(1)
}

logger := log.NewLogEntry()

opts := proxy.NewOptions()
Expand Down