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

Add version command #12

Merged
merged 8 commits into from
Oct 2, 2019
Merged

Add version command #12

merged 8 commits into from
Oct 2, 2019

Conversation

dansimau
Copy link
Contributor

@dansimau dansimau commented Dec 27, 2018

astro version will print the version of astro at the command line.
Version information will be set using ldflags at release time. For more
information, see:
https://stackoverflow.com/questions/11354518/golang-application-auto-build-versioning

Automated releases will be implemented in a separate PR (#14).

@dansimau dansimau self-assigned this Dec 27, 2018
dansimau and others added 3 commits September 27, 2019 14:21
`astro version` will print the version of astro at the command line.
Version information will be set using ldflags at release time. For more
information, see:
https://stackoverflow.com/questions/11354518/golang-application-auto-build-versioning

Automated releasing will be implemented in a separate PR.
@btromanova btromanova force-pushed the add-version-command branch from 54691ae to ec8314c Compare October 2, 2019 10:03
CHANGELOG.md Outdated
Comment on lines 6 to 10
* Propagate SIGINT and SIGTERM to terraform processes (#49)
* Support detach flag with terraform 0.12 (#45)
* Fix plan output for terraform 0.12 (#41)
* Fix bug in initialization of allowed values (#43)
* Don't pass varialbes to modules that don't delcare them (#40)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we put this in a separate PR? They have nothing to do with this one.

(Also, typo: "variables")

@@ -0,0 +1,62 @@
/*
* Copyright (c) 2018 Uber Technologies, Inc.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

2019

@@ -71,10 +71,18 @@ func stringVersionMatches(v string, versionConstraint string) bool {
}

// compiles the astro binary and returns the path to it.
func compileAstro(dir string) (string, error) {
func compileAstro(dir string, ldflags []string) (string, error) {
Copy link
Contributor Author

@dansimau dansimau Oct 2, 2019

Choose a reason for hiding this comment

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

Let's just make this buildFlags []string and allow people to pass arbitrary flags.

Comment on lines 233 to 235
if err != nil {
panic(err)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In tests just do:

require.NoError(t, err)

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

Comment on lines 245 to 247
if err != nil {
panic(err)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

require.NoError(t, err)

Copy link
Contributor

Choose a reason for hiding this comment

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

Done, thanks

assert.Equal(t, "astro version 1.2.3 (ab123) built 2019-01-01T10:00:00\n", stdoutBytes.String())
}

func TestPlanErorrsWithoutConfig(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we test apply too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Added a test below.

@dansimau dansimau removed the request for review from CorgiMan October 2, 2019 10:24
astro/tests/integration_test.go Show resolved Hide resolved
func TestApplyErorrsWithoutConfig(t *testing.T) {
dir, err := ioutil.TempDir("/tmp", "astro-tests")
require.NoError(t, err)
defer os.RemoveAll(dir)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here.

You could also make these two tests into a table test (if you wanted... 😉 )

Copy link
Contributor Author

@dansimau dansimau left a comment

Choose a reason for hiding this comment

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

LGTM

@btromanova btromanova merged commit bc39594 into master Oct 2, 2019
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.

2 participants