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

Add automatic LinkML linting of schema on each PR changing schema #896

Conversation

jfy133
Copy link
Collaborator

@jfy133 jfy133 commented Jan 29, 2025

This adds a github action that runs the LinkML linting if the schema changes, and posts a nicely formatted comment listing all the issues.

Note it currently configures the linting to ignore warnings related to 'standard naming'.

@sujaypatil96 sujaypatil96 self-requested a review January 29, 2025 23:42
Copy link
Collaborator

@sujaypatil96 sujaypatil96 left a comment

Choose a reason for hiding this comment

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

Thanks for opening this PR up into the main branch of the original repo @jfy133.

I've noted a couple of review comments on your PR, we can work on fixing some of these up and we'll try to merge this PR in, in the coming days.

.github/workflows/lint-linkml.yml Outdated Show resolved Hide resolved
.github/workflows/lint-linkml.yml Outdated Show resolved Hide resolved
.github/workflows/lint-linkml.yml Show resolved Hide resolved
.github/workflows/lint-linkml.yml Outdated Show resolved Hide resolved
.github/workflows/lint-linkml.yml Outdated Show resolved Hide resolved
.linkml-lint-config.yml Outdated Show resolved Hide resolved
@sujaypatil96 sujaypatil96 changed the base branch from main to automated-linkml-linting January 30, 2025 00:05
@sujaypatil96
Copy link
Collaborator

I hope you don't mind @jfy133 but I changed the base branch of this PR to another branch called automated-linkml-linting (same name as the branch in your fork). We can merge it into this branch when it's ready, and then we'll open up a PR into main so we can see the GitHub Action workflow in action on that PR into main.

@jfy133 jfy133 requested a review from sujaypatil96 January 30, 2025 07:36
@jfy133
Copy link
Collaborator Author

jfy133 commented Jan 30, 2025

All but one comments addressed @sujaypatil96 !

Feel free to make direct suggestions when commenting (then I can just press commit, rather than having to copy paste) - or even push directly (I'm not precious about this).

Sounds good to moving to an internal testing branch first!

.linkml-lint-config.yml Outdated Show resolved Hide resolved
@jfy133
Copy link
Collaborator Author

jfy133 commented Jan 30, 2025

Should be ready to go @sujaypatil96

@sujaypatil96
Copy link
Collaborator

It's not useful to have a linkml linter config file that simply extends the recommended set of linting rules, it does that by default. Linter config files are useful only if we're deviating from the default (custom), so I think it's okay to delete that file altogether. After that we should be good for approval and merge.

@sujaypatil96
Copy link
Collaborator

sujaypatil96 commented Feb 3, 2025

I've gone ahead and made that change. Once the Actions go through, I'll approve and merge this PR in. Then we can open a PR from the automated-linkml-linting branch to the main branch to see the Action in action 😂.

Copy link
Collaborator

@sujaypatil96 sujaypatil96 left a comment

Choose a reason for hiding this comment

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

Great work on this PR @jfy133! approving and merging it in 🚀

@sujaypatil96 sujaypatil96 merged commit a13f8fe into GenomicsStandardsConsortium:automated-linkml-linting Feb 3, 2025
3 checks passed
@jfy133
Copy link
Collaborator Author

jfy133 commented Feb 3, 2025

Nice! Thanks @sujaypatil96 look forward to the new PR 🤞🤞🤞🤞

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.

2 participants