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 a version flag and other printout updates #428

Merged
merged 13 commits into from
May 7, 2020

Conversation

rafmudaf
Copy link
Collaborator

Complete this sentence
THIS PULL REQUEST IS READY TO MERGE

Feature or improvement description
This pull request adds support for a version flag in the CheckArgs subroutine. Passing the lower case or upper case form of -v or -version will print the version info of the program being run.

The method to get version information was modified to allow a version string to be passed in the command line via CMake. This is useful in cases where the source code is downloaded as a tarball rather than cloned with git. In this case, there is no .git directory and therefore a version isn't automatically set.

Finally, the routines and formatting for printing general information to the console are made more explicit and consistent. A compiler description is also added to the compile information section of the standard printout.

Related issue, if one exists
None

Impacted areas of the software
NWTC Library, drivers and glue codes

Test results, if applicable
GItHub Actions.

@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Mar 26, 2020

Here's an example of the version when the source code is downloaded outside of git: https://github.com/Homebrew/homebrew-core/pull/52107/checks?check_run_id=535445810#step:3:1596

This is an automatic build inside Homebrew. In this case, an OpenFAST binary distributed through that package manager would be unversioned.

Screen Shot 2020-03-26 at 4 18 36 PM

@rafmudaf rafmudaf force-pushed the feature/version_flag branch from 2f99100 to 5aa3e37 Compare March 27, 2020 16:34
@rafmudaf rafmudaf requested a review from andrew-platt March 30, 2020 14:00
Copy link
Collaborator

@andrew-platt andrew-platt left a comment

Choose a reason for hiding this comment

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

This looks reasonable. Have you tested the AD15 driver? I'm not entirely sure I follow all the logic in a quick read through.

@bjonkman
Copy link
Contributor

I haven't read through all of your changes, but am wondering if you have you tried to use the .gitattributes file for some of this? I added the

CreateGitVersion.bat ident export-subst

line in it so that when you download a zip file from GitHub, it replaces a string in the .bat file and Visual Studio builds can still tell us what version it originated with. Might be nice to just have a header file with this info in it so either CMake or Visual Studio could access it.

@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Apr 1, 2020

@bjonkman It hadn't occurred to me to use .gitattributes, but I like the idea of using that to create a version header file and consolidating all of the versioning infrastructure.

@rafmudaf
Copy link
Collaborator Author

rafmudaf commented May 5, 2020

@bjonkman I think the suggestion to create a header file will work, but I haven't seen how to get the latest tag information with gitattributes. This is required in order to recreate out current version. One option is to hard code the latest tag and then append the current git info to the end. In any case, I'd like to think a bit more for a better solution to that.

As for this pull request, do you have any other comments?

@rafmudaf rafmudaf force-pushed the feature/version_flag branch from a14d267 to f1a9bc2 Compare May 5, 2020 17:33
@rafmudaf rafmudaf marked this pull request as draft May 5, 2020 18:37
@rafmudaf rafmudaf marked this pull request as ready for review May 6, 2020 00:00
@rafmudaf rafmudaf merged commit bf89cfc into OpenFAST:dev May 7, 2020
@rafmudaf rafmudaf deleted the feature/version_flag branch May 7, 2020 16:08
@rafmudaf rafmudaf mentioned this pull request Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants