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

Implement fj-app --version #850

Closed
hannobraun opened this issue Jul 20, 2022 · 3 comments · Fixed by #868
Closed

Implement fj-app --version #850

hannobraun opened this issue Jul 20, 2022 · 3 comments · Fixed by #868
Labels
type: feature New features and improvements to existing features

Comments

@hannobraun
Copy link
Owner

fj-app should have a --version argument that returns the current version. Given that we release weekly, it would be great if this version wouldn't have to be updated manually. This should be straight-forward to achieve: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates

While I would definitely merge a pull request that implements this straight-forward version, it would be great if there was some additional nuance:

  • The ID of the Git commit that the binary was built from should be output as well.
  • It should be clear whether the binary is from a published release or not.

I assume that both of those can be achieved by creating the version string in a build script, and passing it to the fj-app source code as an environment variable. The release automation could use an environment variable to signal the build script whether this is an official release build.

@hannobraun hannobraun added type: feature New features and improvements to existing features topic: ui labels Jul 20, 2022
@hannobraun hannobraun added this to the Basic Usability milestone Jul 20, 2022
@hendrikmaus
Copy link
Contributor

hendrikmaus commented Jul 28, 2022

Clap has the long version for this and a convenience Macro to help setting it during compile time https://docs.rs/clap/latest/clap/builder/struct.App.html#method.long_version

To figure out if we are building a release or not, release-operator's deduce step must run first.

I would propose to add a job that runs before the binary build job. It only runs the deduce command and shares the outcome as an output with the remainder of the pipeline. Then the binary builder job can wait for that and use the output.

And after building the binaries, the remaining release-operator job also runs conditionally on the output from the first job that ran the deduce command.

I don't think you need a build.rs or anything more than Clap and a bit of pipeline step reordering.

@hannobraun
Copy link
Owner Author

Thank you for the insight, @hendrikmaus! I've opened #883 (probably while you were typing this 😄), where I suggest something very similar to the reordering you suggest here.

Regarding using Clap's capabilities instead of a build.rs, I'd be happy to merge any improvements or simplifications!

@hannobraun
Copy link
Owner Author

On second thought, we might want to use the version in other parts of the UI, not just as a response to --version. We might even want to have it available in other applications in the future, that don't rely on Clap. Or maybe we'll have multiple CLI applications that all want to display the same version.

In any case, it might be a disadvantage to rely on Clap for this capability. Unless the current build.rs-based solution causes any maintenance issues, I think it makes sense to stay with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature New features and improvements to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants