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

oeo-closure.owl no longer built on every push #1955

Merged
merged 1 commit into from
Nov 10, 2024

Conversation

ColinHDev
Copy link
Contributor

@ColinHDev ColinHDev commented Oct 20, 2024

Summary of the discussion

The OEO CI workflow is run on every push and regularly requires around 11 minutes to build and test the ontology. To do this it builds the oeo-full.omn, oeo-full.owl and oeo-closure.owl files. However, only the first two are required for the subsequent test step.

This Pr changes the workflow to no longer build the oeo-closure.owl file as it does not need to be built after every push. As can be seen at the required completion time on the workflow of this pr, the time was reduced to 3m 39s.

This breaks backwards compatibility as this workflow also attaches the built files as artifacts of the workflow run for anyone to download. So any script, tool or tutorial (e.g. the wiki entry How-to-release-a-new-ontology-version) will no longer be feasible since the artifacts no longer contain the previously expected files.
The problem with the release workflow as described in the tutorial is fixed with #1956.

Type of change (CHANGELOG.md)

---

Workflow checklist

Automation

Closes #1934

PR-Assignee

Reviewer

  • 🐙 Follow the Reviewer Guide
  • 🐙 Provided feedback and show sufficient appreciation for the work done

@ColinHDev ColinHDev requested a review from stap-m October 20, 2024 21:02
@ColinHDev ColinHDev assigned ColinHDev and unassigned ColinHDev Oct 20, 2024
@ColinHDev ColinHDev changed the title owl-closure.owl no longer built on every push oeo-closure.owl no longer built on every push Oct 20, 2024
@ColinHDev
Copy link
Contributor Author

Since a few people were added to review the PR, here is a little explanation of why changing this one line (hopefully) works as expected:

  1. The make command builds the ontology using the Makefile, which defines all the steps to build the ontology.
  2. In those Makefiles you can also define so-called targets.
    • This is the base target, which we could run specifically by executing make base.
    • Here is the all target. It is defined that if it is run by executing make all, it will call the base, merge, and then closure targets.
    • You can execute a given or multiple targets by defining them after the make command. If no target is given, the make command will execute the first target in the file.
  3. In the previous implementation, only make was called, so it would execute the first target in the Makefile, which is all. This target then calls base, merge and closure.
  4. I am yet not really sure, how to describe what the base target exactly does. But the merge target also builds the oeo-full.owl file, while the closure target builds the oeo-closure.owl.
  5. So I changed that line to no longer implicitly call the base merge closure targets, but explicitly only the base merge targets. This way it no longer builds the oeo-closure.owl.

I hope this helps. :)

@stap-m
Copy link
Contributor

stap-m commented Oct 28, 2024

As addition to @ColinHDev explanation: the oeo-closure will be removed from the usual push workflows. With #1956 its creation will be only performed in the post-release procedures.

Copy link
Contributor

@madbkr madbkr left a comment

Choose a reason for hiding this comment

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

I am by no means an expert on makefiles but from what I read up that change should work as wanted.

@ColinHDev ColinHDev merged commit 4fb136c into dev Nov 10, 2024
4 checks passed
@ColinHDev ColinHDev deleted the feature-1934-push-workflow-time-reduction branch November 10, 2024 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Build oeo-closure only during release process
3 participants