Conversation
dennisklein
force-pushed
the
generate-codemeta
branch
3 times, most recently
from
86448d9 to
8bb6061
Compare
dennisklein
force-pushed
the
generate-codemeta
branch
from
8bb6061 to
6bc5000
Compare
dennisklein
force-pushed
the
generate-codemeta
branch
2 times, most recently
from
54cb07f to
ed16872
Compare
dennisklein
force-pushed
the
generate-codemeta
branch
from
ed16872 to
ff8079b
Compare
ChristianTackeGSI
left a comment
ChristianTackeGSI
left a comment
There was a problem hiding this comment.
Okay. Some comments.
If you want to fix some of them later and consider this a "I need testing for tags" thing, then that's fine.
| manipulator.save() | ||
| filename = None | ||
| if args.outdir is not None: | ||
| filename = f'{args.outdir}/{manipulator.default_filename}' |
There was a problem hiding this comment.
I would prefer a pathlib.Path based approach for this. Yes, not needed for us. But still nicer?
| endif() | ||
|
|
||
| add_custom_target(codemeta | ||
| python3 ${CMAKE_SOURCE_DIR}/meta_update.py --outdir ${CMAKE_BINARY_DIR} --set-version ${PROJECT_VERSION} |
There was a problem hiding this comment.
I would expect a find_package(2)(Python … something interpreter) for this, right?
There was a problem hiding this comment.
Ya, was considering it, but there was so much noise with CMake's Python detection in the past that I am afraid it will introduce more maintenance work than hardcoding it - this target really does not need to be super platform independent.
There was a problem hiding this comment.
Okay, got that! Please add a comment above it to that fact. Something like "meant for release management, not needed to be as portable".
Have you looked into add_custom_command(OUTPUT … MAIN_DEPENDENCY …) or the SOURCES part of add_custom_target?
| publish: | ||
| needs: validate |
There was a problem hiding this comment.
Okay, now you have the dep. Maybe do the moving of the validate step in this commit instead?
| paths: | ||
| - codemeta.json | ||
| - .github/workflows/codemeta.yaml | ||
| tags: | ||
| - 'v*' |
There was a problem hiding this comment.
This changes the semantics (AND) compared to the "old" validate workflow. Please comment on this in the commit text body. :-)
There was a problem hiding this comment.
thx, ye, will revisit this. thought it was OR logic.
| - name: generate codemeta.json | ||
| run: cmake --build build --target codemeta | ||
| - name: print result | ||
| run: cat build/codemeta.json |
There was a problem hiding this comment.
Maybe?
| run: cat build/codemeta.json | |
| run: diff codemeta.json build/codemeta.json |
2 participants
For #482. Replacing zenodo release pull with github push on release is an exercise for another day.
(Once we merge this, I will make a tag to see if the asset upload works. So far untested.)