Skip to content

Conversation

@Karan-Palan
Copy link
Contributor

@Karan-Palan Karan-Palan commented Sep 2, 2025

🧐 Context

  1. Removed Anti-Patterns:

    • Removed type alongside enum as the enumeration choices already imply their respective types.
    • Removed type alongside const as the constant already implies its respective type.
  2. Resolved Redundant Keywords:

    • Removed additionalItems where items is set to a schema, as it is ignored in such cases.
    • Removed minContains where contains is not present, as it is meaningless without contains.
  3. Optimized Schema Structure:

    • Removed unnecessary allOf wrappers to improve evaluation

Please review the changes and provide feedback if necessary.

🤙 How to test

  • Ran jsonschema lint after the fixes to ensure no further issues were reported.
  • Verified the schema functionality remains intact.

Before:

image

After:

image

Note

I, along with Juan (JSON Schema TSC member) are defining linting rules for JSON Schema as a Part of a GSoC (Google Summer of code) project here - https://github.com/Karan-Palan/JSON-Schema-Linting, and implementing their auto-fixes here - https://github.com/sourcemeta/jsonschema/blob/main/docs/lint.markdown. We have recently added many rules prefixing unknown keywords with x- which will be introduced in the newer JSON Schema drafts
If beneficial to the project, I suggest integrating the complete cli with it to write the best schemas and catch any errors and follow best practices. Examples of integration - https://github.com/krakend/krakend-schema/blob/main/.github/workflows/validate-json-schema.yml#L10, daveshanley/vacuum#701

@recacha recacha self-assigned this Sep 5, 2025
@recacha recacha requested review from javalon and recacha September 5, 2025 09:52
@recacha
Copy link
Contributor

recacha commented Sep 5, 2025

Hi Karan.

Thank you so much for your contribution to our repository. We truly appreciate the time and effort you've dedicated to improving it.

We've reviewed the pull request, and it looks like there might be some spacing format changes that are causing all lines to appear as modified in the diff. Would it be possible for you to adjust the formatting so that the diff only reflects the actual changes you've made? This would greatly help us in reviewing and merging your valuable contribution.

Thank you again for your understanding and for your efforts.

jviotti added a commit to sourcemeta/jsonschema that referenced this pull request Sep 5, 2025
jviotti added a commit to sourcemeta/jsonschema that referenced this pull request Sep 5, 2025
@jviotti
Copy link

jviotti commented Sep 5, 2025

Hi @recacha @Karan-Palan ,

Great feedback on the formatting! I just released v11.8.0 (https://github.com/sourcemeta/jsonschema/releases/tag/v11.8.0) which implements an --indentation option to lint --fix to tell it to respect the current 4-spaces indentation. I think that will already make the diff much nicer, and happy to add more tweaks if needed.

@Karan-Palan Also, I think some of the diff is because you are also running jsonschema fmt. Maybe do that in another PR, if any?

@Karan-Palan
Copy link
Contributor Author

Karan-Palan commented Sep 6, 2025

@jviotti, thank you for the quick updates! That makes the diff much nicer. I'm updating the PR now. Also, @recacha, the jsonschema fmt command ensures that schemas are formatted to the highest standard. It is used in many projects, example being Krakend. If you would want to give it a try, I'll send a PR for that!

@recacha
Copy link
Contributor

recacha commented Sep 8, 2025

Hi @Karan-Palan and @jviotti

I really appreciate your contributions and the excellent work!

Regarding the indentation correction, I would be very grateful if, as Juan mentioned, this could be added in a new pull request :-)

Thank you once again!!

@recacha recacha merged commit a072091 into getmanfred:main Sep 8, 2025
2 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.

3 participants