Skip to content

Extend checks and add pre-commit to CI #410

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Extend checks and add pre-commit to CI #410

wants to merge 1 commit into from
+10 −11

Conversation

vladistan
Copy link
Contributor

No description provided.

dalito
dalito reviewed Jul 18, 2025
View reviewed changes
.github/workflows/main.yaml Outdated
quality-checks:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4.2.2
- uses: actions/checkout@4.2.2
Copy link
Member

@dalito dalito Jul 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The version tag for the release of this action contains a "v".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But the workflow in linkml does not use v. I think we should have consistent style between both repos.

https://github.com/linkml/linkml/blob/56dd784d7794b9958f10f1a740e05e28186f0065/.github/workflows/main.yaml#L18

Copy link
Member

Choose a reason for hiding this comment

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

Ok. If it works, fine! With the next action update dependabot will make the spelling decision. 😉

Copy link
Member

Choose a reason for hiding this comment

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

The version without "v" stopped working in linkml: linkml/linkml#2865 - So we should also use the exact tag name here (current is v5.0.0).

@dalito
Copy link
Member

dalito commented Jul 18, 2025 •
edited
Loading

I'll merge #406 and we can improve with this one (like hhtps in link to editoconfig site) and add its tighter checks.

@dalito dalito changed the title Add checks and pre commit Extend checks and add pre-commit to CI Jul 18, 2025
@vladistan vladistan force-pushed the add-checks-and-pre-commit branch 2 times, most recently from d0fa10b to 3fc6d94 Compare July 18, 2025 20:06
@vladistan
Copy link
Contributor Author

vladistan commented Jul 18, 2025 •
edited
Loading

So this one has two parts. First is fixing up config files like .editorconfig and main workflow, making them more similar to their counterparts in linkml repo. Second part is tailoring quality checks to the runtime, and also making checks be similar to linkml, like performing full pre-commit run on all files instead of just running ruff and codespell on python files.

I am thinking about splitting it into two smaller PRs. Although this PR is not particularly large. What do you think?

@vladistan vladistan force-pushed the add-checks-and-pre-commit branch from 3fc6d94 to 6110767 Compare July 18, 2025 20:51
@vladistan vladistan marked this pull request as draft July 18, 2025 20:57
@vladistan
Copy link
Contributor Author

Ok, I split this into two PRs. Turning this one into draft until we fix all problems highlighted by the lint

@vladistan vladistan force-pushed the add-checks-and-pre-commit branch 5 times, most recently from c950d43 to 8fe0d1c Compare July 22, 2025 22:57
@vladistan vladistan force-pushed the add-checks-and-pre-commit branch 4 times, most recently from 2ebe78f to 886394d Compare August 4, 2025 21:20
@vladistan vladistan force-pushed the add-checks-and-pre-commit branch from 886394d to da8647e Compare August 18, 2025 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dalito dalito dalito left review comments

@kevinschaper kevinschaper kevinschaper 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.
3 participants
@vladistan @dalito @kevinschaper