-
Notifications
You must be signed in to change notification settings - Fork 17
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
basic repo maintenance #167
Conversation
…en authoring schemas
… legacy setup.py and setup.cfg
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.
Hahah woops, a couple minutes late on this review 😂 But just a couple of minor comments.
on: | ||
pull_request: | ||
branches: [ main ] | ||
types: [ opened, synchronize, reopened ] |
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 think it's fine to explicitly specify the activity types here, but just FYI these are the default anyway:
By default, a workflow only runs when a
pull_request
event's activity type isopened
,synchronize
, orreopened
. To trigger workflows by different activity types, use the types keyword.
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
steps: | ||
- uses: actions/checkout@v2 | ||
- name: Validate YAML file | ||
run: yamllint -c .yamllint-config linkml_model/model/schema/*.yaml |
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.
How does yamllint
get installed in this workflow?
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.
yamllint is already installed on Ubuntu-based GitHub runners.
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.
Whoa! actions/runner-images#1142 So it is. The more you know!
poetry run codespell | ||
|
||
lint: | ||
poetry run yamllint -c .yamllint-config linkml_model/model/schema/*.yaml |
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.
What do you think about using one of the three standard config file names (.yamllint
, .yamllint.yaml
, or .yamllint.yml
-- https://yamllint.readthedocs.io/en/stable/configuration.html) so that the -c
flag doesn't have to be specified every time?
| [subsets](subsets.md) | 0..* <br/> [SubsetDefinition](SubsetDefinition.md) | An index to the collection of all subset definitions in the schema | | ||
| [prefixes](prefixes.md) | 0..* <br/> [Prefix](Prefix.md) | prefix / URI definitions to be added to the context beyond those fetched from... | | ||
| [default_prefix](default_prefix.md) | 0..1 <br/> [xsd:string](http://www.w3.org/2001/XMLSchema#string) | default and base prefix -- used for ':' identifiers, @base and @vocab | | ||
| [default_range](default_range.md) | 0..1 <br/> [TypeDefinition](TypeDefinition.md) | default slot range to be used if range element is omitted from a slot definit... | |
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 probably tangential to your changes, but what's up with all these tables with truncated lines? Are these autogenerated from something?
yep, those lines are autogenerated. Maybe better just to link out.
… Message ID: ***@***.***>
|
gen-project
which doesmeta.yaml
with-mergeimports=False
)..HTML
files. I removed that regex and limited it by file extension instead. I added a makefile target to run codespell locally and cleaned up the reported errors.yaml
linter to the GH actions and a Makefile target to run it. The next step is using our own linkml-lint to do the equivalent level of linting for these older schemas that might take awhile to move to the new standards (e.g. Biolink, linkml-model, etc.).codespell
,yamllint
dev dependencies inpyproject.toml
. I know there is more to do in the pyproject.toml file - I stopped short of doing the rest!setup.py
andsetup.cfg
any longer. These are also removed in this PR.linkml:notes
to all our Types classes to indicate on our generated doc, that we should use lowercase names for these instead of uppercase.