-
Notifications
You must be signed in to change notification settings - Fork 56
Changelog Generation with release_changelog #1249
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
base: master
Are you sure you want to change the base?
Changes from all commits
fe42595
0b3d86c
72c1d49
0a4094f
c4b5a85
af4d57f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1556,6 +1556,28 @@ update_docs: | |
| mkdocs gh-deploy --config-file ../../mkdocs.yaml | ||
| {%- endif %} | ||
|
|
||
| .PHONY: release_changelog | ||
| release_changelog: $(REPORTDIR)/changelog.md $(REPORTDIR)/changelog.csv $(REPORTDIR)/changelog.yaml | ||
|
|
||
| $(TMPDIR)/latest_release.obo: $(TMPDIR) | ||
gouttegd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| wget $(ONTBASE)/$(ONT)-base.owl -O [email protected] || { \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if the dependence on "base" here should be documented somewhere. Not everyone likes the idea of having a base, but everyone will want to generate release notes.. Can we make the "diff" ontology configurable, using base by default if and only if it is a configured release product, otherwise the main release? |
||
| echo "Latest release couldn't be found, creating an empty OBO file instead"; \ | ||
| echo "format-version: 1.2" > [email protected]; \ | ||
| } | ||
| $(ROBOT) convert -i [email protected] --check false -f obo $(OBO_FORMAT_OPTIONS) -o $@ && rm [email protected] | ||
|
|
||
| $(TMPDIR)/current_release.obo: $(ONT)-base.owl | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is good as it is not at all certain the ontology does indeed have an OBO release. |
||
| $(ROBOT) convert -i $< --check false -f obo $(OBO_FORMAT_OPTIONS) -o $@ | ||
|
|
||
| $(REPORTDIR)/changelog.md: $(TMPDIR)/latest_release.obo $(TMPDIR)/current_release.obo | ||
| runoak -i simpleobo:$< diff -X simpleobo:$(word 2, $^) -o $@ --output-type md | ||
|
|
||
| $(REPORTDIR)/changelog.csv: $(TMPDIR)/latest_release.obo $(TMPDIR)/current_release.obo | ||
| runoak -i simpleobo:$< diff -X simpleobo:$(word 2, $^) -o $@ --output-type csv --statistics --group-by-property oio:hasOBONamespace | ||
|
|
||
| $(REPORTDIR)/changelog.yaml: $(TMPDIR)/latest_release.obo $(TMPDIR)/current_release.obo | ||
| runoak -i simpleobo:$< diff -X simpleobo:$(word 2, $^) -o $@ --output-type yaml | ||
|
|
||
gouttegd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| # Note to future generations: computing the real path relative to the | ||
| # current directory is a way to ensure we only clean up directories that | ||
| # are located below the current directory, regardless of the contents of | ||
|
|
@@ -1627,6 +1649,7 @@ Additional build commands (advanced users) | |
| * all_assets: Build all assets | ||
| * show_assets: Print a list of all assets that would be build by the release pipeline | ||
| * all_mappings: Update all SSSOM mapping sets | ||
| * release_changelog: Generates a markdown changelog report summarizing ontology changes along with KGCL outputs. This function converts OWL ontologies to OBO format for diffing, which may result in information loss due to OBO's limited expressivity compared to OWL. | ||
|
|
||
| Additional QC commands (advanced users) | ||
| * robot_reports: Run all configured ROBOT reports | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open question: for now, the release changelogs are only generated “on demand”, when a user explicitly runs
make release_changelog.Do we want the changelog to be generated automatically? The original issue is uncommitted about that that.
(Personally I’d say no – i.e., leave things like that, generate on-demand only –, or at least, if it is generated automatically make it configurable.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd favour to be able to activate it via the config yaml. Running it on demand only would suffice, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My worry is if you do NOT fold it into the release process, then some quirk in the dependency chain might cause a complete regeneration of the "current release" locally, while its 99% certain that, if the rule is indeed run manually, the release has already been generated.
Also, if the temporary release files are copied up during prepare_release, it will definitely regenerate the release.
So I vote for
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only happen if a source file has changed since the last time the release was built – and in that case, re-generating the release before making the diff is the expected behaviour.
If a dependency on
$(ONT)-base.owlcauses$(ONT)-base.owlto be rebuilt even though no source has changed, then it’s not a “quirk in the dependency chain”, it’s a full-on bug – one that should be fixed once it is reported.I don’t see why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See here:
ontology-development-kit/template/src/ontology/Makefile.jinja2
Lines 433 to 439 in d85794b
People run:
prepare_release; clean will clean up main release files so they are no longer in src/ontology. Running the diff will force the base to be rebuild.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I would consider a bug.
prepare_releaseshould not, in my opinion, clean files after a release.What is even the point of doing that? It only ensures that the next time
prepare_releaseis run, it would be from a clean state.It would be much useful to ensure that intermediate files are cleaned up before running the release (that’s in fact what I always do: I never run
make prepare_release, alwaysmake clean prepare_release).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⬆️ Anyway, this above is a different issue, and one that I am reluctant to fix now, so close to release – this “cleaning after release“ has been the behaviour of
prepare_releasefor a while, I don’t think it is urgent to change it (I do think it should be changed, just not now).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For simplicity, I’d favour a single option, that decides both whether a changelog is generated and the type of artefact from which it is computed (instead of one option to enable the changelog and another one from the type of artefact).
Something like
where
noneis understood as disabling the production of the changelog.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me!