Skip to content

Conversation

vivodi
Copy link
Contributor

@vivodi vivodi commented Sep 4, 2025

No description provided.

@vivodi vivodi force-pushed the codecov branch 2 times, most recently from d306c75 to 7f26f9b Compare September 4, 2025 11:28
Copy link
Member

@sirosen sirosen left a comment

Choose a reason for hiding this comment

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

Sorry that review took so long here -- I've been on a tight schedule lately for FOSS work.

I have one thing I want to clarify before accepting, but the change looks well-formed. Thanks again for all of these new schemas and hooks!

"hook_config": {
"name": "Validate Codecov config",
"files": [
r"^((\.github|dev)/)?\.?codecov\.ya?ml$",
Copy link
Member

Choose a reason for hiding this comment

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

Can you give me a pointer to codecov docs for this which indicate that .github/codecov.yaml is supported?

The reason I raise this is that I prefer to follow the lead of SchemaStore whenever possible (i.e. as long as we don't have reason to believe that SchemaStore is wrong). It makes for a much simpler policy around the file paths.

SchemaStore currently lists only codecov.yml and .codecov.yml in the repo root. If that's wrong, I can accept this PR and work on upstreaming the improvement there (or you can give them a PR if you like -- I've always found them very friendly!).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://docs.codecov.com/docs/codecov-yaml#can-i-name-the-file-codecovyml

Please submit a PR to the upstream; I don’t intend to submit one.

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thanks! I'll make sure your work finds its way up into SchemaStore.

@sirosen sirosen merged commit e75ba21 into python-jsonschema:main Sep 18, 2025
23 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.

2 participants