Skip to content
This repository has been archived by the owner on Oct 29, 2021. It is now read-only.

Add version command #25

Merged
merged 1 commit into from
Mar 22, 2021
Merged

Conversation

Ashmita152
Copy link
Contributor

@Ashmita152 Ashmita152 commented Mar 22, 2021

Signed-off-by: Ashmita Bohara [email protected]

Closes: #5

Done as per cobra documentation: https://github.com/spf13/cobra#version-flag

After this fix:

$ ./opentelemetry-collector-builder version
dev

@Ashmita152 Ashmita152 requested review from a team March 22, 2021 06:05
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 22, 2021

CLA Signed

The committers are authorized under a signed CLA.

cmd/root.go Outdated
@@ -61,12 +62,12 @@ func Execute() {
// the distribution parameters, which we accept as CLI flags as well
cmd.Flags().StringVar(&cfg.Distribution.ExeName, "name", "otelcol-custom", "The executable name for the OpenTelemetry Collector distribution")
cmd.Flags().StringVar(&cfg.Distribution.LongName, "description", "Custom OpenTelemetry Collector distribution", "A descriptive name for the OpenTelemetry Collector distribution")
cmd.Flags().StringVar(&cfg.Distribution.Version, "version", "1.0.0", "The version for the OpenTelemetry Collector distribution")
Copy link
Member

Choose a reason for hiding this comment

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

This field is about the version to use for the underlying build. Instead of removing, this needs to have a new name, which will cause a breaking change.

Copy link
Contributor Author

@Ashmita152 Ashmita152 Mar 22, 2021

Choose a reason for hiding this comment

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

It is confusing and I somehow thought otelcol-version is the version of the underlying collector binary. I am still confused. what are the version and otelcol-version fields referring to ? We would need to give them better nomenclature.

Copy link
Member

Choose a reason for hiding this comment

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

This repository builds custom distributions of the OpenTelemetry Collector, and the distribution itself might have its own version. Imagining that Jaeger would be such a distribution, version would be Jaeger's version (like 2.0.0-alpha1), and otelcol-version is which version of OpenTelemetry Collector version to base on (0.22.0, for instance)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you Juraci for the explanation. I think I get it now. This means we have to deal with 3 versions variables in total. In order to prevent making breaking change, I opted to go ahead with a subcommand:

$ ./opentelemetry-collector-builder version
dev

Let me know in case you think we should do it in some other way.

@Ashmita152 Ashmita152 changed the title Fix --version flag Add version command Mar 22, 2021
Signed-off-by: Ashmita Bohara <[email protected]>
@jpkrohling jpkrohling merged commit a6b22d3 into open-telemetry:main Mar 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to just print the version
2 participants