-
Notifications
You must be signed in to change notification settings - Fork 52
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 #84
Conversation
❌ Author of the following commits did not sign a Contributor Agreement: Please, read and sign the above mentioned agreement if you want to contribute to this project |
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
// KIND, either express or implied. See the License for the | ||
// specific language governing permissions and limitations | ||
// under the License. |
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.
The project is licensed under Apache License v2.0 so it doesn't look right but many files are like that. I will check if this is normal or not and I will correct it if necessary in another pr.
if !ok { | ||
fmt.Println("unknown") | ||
} else { | ||
fmt.Println(info.Main.Version) |
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.
I'm discovering this. According to golang/go#50603, it will work only with go install
(it doesn't on my machine). Isn't that a strong limitation?
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.
yes but i imagine thats how many people consume it, for example https://github.com/elastic/cloud-on-k8s/blob/2.8/hack/api-docs/build.sh#L35
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.
oh i meant to say also that this is painful to test because of that. like it needs to come from a go install
of the official repo or doing some fancy acrobatics. BUT you can see it in other repos too
https://github.com/search?q=%22info.Main.Version%22&type=code
@thbkrkr @anyasabo According to golang/go#51831, we can read
instead of
Additionally, I have implemented version output in #78, we could discuss which approach to use? If deciding to use |
I don't know what I'm missing but I haven't managed to get a version other than |
Given the limited cases where the version is correctly propagated, I'm more in favor of the "old" approach proposed in #78. |
I'd be happy to revisit the version command with In the meantime, I merged #78 in favor of this one, hence closing. |
Hi old friends,
I beat my head against an issue for a while today, and it turned out because I had an older version of this CLI installed than my colleagues. We pin other dependencies but this one does not expose its version easily, so this adds a version command so we can easily ensure the desired version is installed. Hope you all are at least okay.
Anya