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

Chore: customize github workflows to correctly install system dependencies #151

Merged
merged 14 commits into from
Jan 16, 2025

Conversation

m-kolomanski
Copy link
Collaborator

@m-kolomanski m-kolomanski commented Jan 3, 2025

Issue

Closes #149

Also resolves issues with checks in PR #146

Description

Reworks and customizes github workflows. Instead of relying on reusable workflows provided by admiralci, each of the needed workflows is inlined in the main workflow. This means that the jobs can be customizable in terms of correct installing of system deps, as well as efficiency. The drawback being that the workflows will need periodical, manual maintenance when updates are available.

How to test

Currently the PR also changes the DESCRIPTION, adding the units package, which has problematic system dependencies. With the changes, all checks pass. Before merging, changes to the DESCRIPTION file should be removed.

@m-kolomanski m-kolomanski changed the title chore: restored spellcheck Chore: customize github workflows to correctly install system dependencies Jan 7, 2025
@m-kolomanski m-kolomanski marked this pull request as ready for review January 7, 2025 13:50
@Gotfrid
Copy link
Collaborator

Gotfrid commented Jan 8, 2025

Thank you for making this change, and confirming that it works!

Considering that #146 is still in review, let's wait - maybe we get some meaningful suggestions in pharmaverse/admiralci.

If #146 is ready before we get an answer, then let's proceed as we discussed - copy over the workflow files, modify them to install dependencies, and reference them from the main workflow.

@m-kolomanski m-kolomanski force-pushed the chore/customize-workflows branch 2 times, most recently from 3a72434 to bdcc4b2 Compare January 9, 2025 08:01
@m-kolomanski m-kolomanski force-pushed the chore/customize-workflows branch from bdcc4b2 to c52bbec Compare January 9, 2025 08:02
@m-kolomanski m-kolomanski force-pushed the chore/customize-workflows branch from 28dcd67 to ec4dec7 Compare January 9, 2025 08:29
@m-kolomanski
Copy link
Collaborator Author

@Gotfrid FYI, I have implemented the discussed workflow structure (copied files over, added minimal modifications, referenced in main file) and verified that everything works correctly with units package, the dependencies are being installed.

Copy link
Collaborator

@Gotfrid Gotfrid left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@m-kolomanski
Copy link
Collaborator Author

So just for the record, we are now waiting for a response from admiralci to see if there is a better way of achieving that result.

Currently, this PR is blocking #146, as the checks are failing, but it is still in review. If #146 passes the review and we do not get the response, I will merge this PR to unblock #146 .

@m-kolomanski
Copy link
Collaborator Author

It does not look like we are going to get any meaningful answer. I am merging the PR. If any better solution arises, we can go back to this issue.

@m-kolomanski m-kolomanski merged commit 430c969 into main Jan 16, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chore: Install system dependencies of units package in github workflows
3 participants