Skip to content

Linter: create rule to remove empty fragment #1825

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

Merged

Conversation

Karan-Palan
Copy link
Contributor

@Karan-Palan Karan-Palan commented Jul 8, 2025

No description provided.

Signed-off-by: karan-palan <karanpalan007@gmail.com>
ModernOfficialDialectWithEmptyFragment()
: SchemaTransformRule{
"modern_official_dialect_with_empty_fragment",
"The official dialect URI of Draft 2019-09 and newer versions must "
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"The official dialect URI of Draft 2019-09 and newer versions must "
"The official dialect URI of 2019-09 and newer versions must "

We don't say draft anymore for the newer ones!

@jviotti
Copy link
Member

jviotti commented Jul 10, 2025

Looks good and looking forward to the tests!

Signed-off-by: karan-palan <karanpalan007@gmail.com>
Signed-off-by: karan-palan <karanpalan007@gmail.com>
@Karan-Palan Karan-Palan changed the title [WIP] feat(alterschema): [linter] create rule to remove empty fragment Linter: create rule to remove empty fragment Jul 14, 2025
@Karan-Palan
Copy link
Contributor Author

@jviotti , I had to make a few change in the official schema resolver to make the tests work. Ptal

@Karan-Palan Karan-Palan marked this pull request as ready for review July 14, 2025 17:18
@jviotti
Copy link
Member

jviotti commented Jul 14, 2025

Can you send a separate PR with the official resolver changes? Also we should never be modifying the actual schemas in there, i.e. setting the $id accordingly.

R"EOF(@METASCHEMA_JSONSCHEMA_2020_12@)EOF");
schema.at("$id").into(sourcemeta::core::JSON{identifier});
Copy link
Member

Choose a reason for hiding this comment

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

We should never mutate the official meta-schemas, as they are supposed to be immutable. What was the intention?

@jviotti
Copy link
Member

jviotti commented Jul 14, 2025

Ah, I see the assert you are hitting in vocabularies(). I think we should instead change the assertion to parse as URI, canonicalise both, and compare them?

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
Copy link
Member

@jviotti jviotti left a comment

Choose a reason for hiding this comment

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

I added a commit to tweak the assertion instead!

@jviotti jviotti merged commit e620979 into sourcemeta:main Jul 14, 2025
14 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