8000 Add CMakeGitVersion to auto-fill covfie_VERSION from git metadata by sethrj · Pull Request #83 · acts-project/covfie · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add CMakeGitVersion to auto-fill covfie_VERSION from git metadata #83

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.

Sign up for GitHub

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 8000 to your account

Jump to bottom
Merged
merged 7 commits into from
May 6, 2025
Merged

Add CMakeGitVersion to auto-fill covfie_VERSION from git metadata #83

merged 7 commits into from
May 6, 2025

Conversation

sethrj
Copy link
Contributor
@sethrj sethrj commented Apr 25, 2025

Even though the latest version of Covfie was released through git as v0.13, the CMake metadata remains at 0.12.1 (and thus the version reported from a spack install is wrong) . I have added https://github.com/sethrj/cmake-git-version which

Issues for this PR because it won't lint:

  • I'm not sure if/how the license of this product should be updated in the REUSE
  • REUSE lint also complains a bout missing license in .gitattributes (really?)
  • CMake lint complains about missing functions, which there aren't (so I may be misunderstanding it)
  • Is there a way to exclude the CMake file from formatting, so it can be diffed against the main version upstream?

@stephenswat
Copy link
Member

Thanks @sethrj, very nice contribution. Before we talk about the actual workings of this CMake code, let's fix the linting:

I'm not sure if/how the license of this product should be updated in the REUSE

The license of this new CMake file should follow the license that you picked for it when you implemented it, so Apache-2.0; that seems to have worked well, and REUSE is not complaining. However, REUSE requires a copy of every license found across the software to be included in the repo. To do this automatically, just run reuse download. That will automatically download the Apache 2.0 license into the repo.

REUSE lint also complains a bout missing license in .gitattributes (really?)

To fix this, edit REUSE.toml and add * 8000 */.gitattributes to the annotation marking code as CC0-1.0.

CMake lint complains about missing functions, which there aren't (so I may be misunderstanding it)

This is only a warning, no need to worry about it.

Is there a way to exclude the CMake file from formatting, so it can be diffed against the main version upstream?

I think you can put # gersemi: off in a comment at the top and # gersemi: on at the bottom. That's still a small diff but much less than actually formatting the file.

@sethrj
Copy link
Contributor Author
sethrj commented Apr 25, 2025

Thanks @stephenswat ! Since CGV embeds its own metadata on the bottom I think it's OK to have it reformatted to match the target repo's style.

sethrj
sethrj commented Apr 25, 2025
View reviewed changes
@sethrj sethrj mentioned this pull request May 2, 2025
Closed
stephenswat
stephenswat approved these changes May 6, 2025
View reviewed changes
Copy link
Member
@stephenswat stephenswat left a comment

Choose a reason for hiding this comment

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

Sorry for the delay; I took a look and it seems like this works like a dream! Thank you!

89DF
@stephenswat stephenswat merged commit 4b54067 into acts-project:main May 6, 2025
13 checks passed
@sethrj sethrj deleted the cmake-git-version branch May 7, 2025 06:28
@sethrj sethrj mentioned this pull request May 8, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephenswat stephenswat stephenswat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sethrj @stephenswat
0