Skip to content

Conversation

@chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Dec 16, 2025

This pull request refactors how field schemas are handled for extra options and core fields in the sphinx_needs extension. The main change is moving from separate type and item_type attributes to a unified schema attribute, which supports more flexible and robust validation using JSON Schema. Error handling is also improved to raise clear exceptions on misconfiguration. Several now-unnecessary validation utilities are removed, and the code is updated to consistently work with the new schema structure.

This will make it easier to implement #1547

Schema handling and validation improvements:

  • Replaced type and item_type attributes in FieldSchema with a single schema attribute, and updated all relevant logic to use and validate this schema using JSON Schema. This allows for more flexible and accurate representation of field types, including arrays and nested types. (sphinx_needs/needs_schema.py, sphinx_needs/needs.py) [1] [2]
  • Updated FieldSchema.__post_init__ to validate the schema using validate_extra_option_schema and validate_object_schema_compiles, raising a clear error if the schema is invalid. (sphinx_needs/needs_schema.py)
  • Added type and item_type properties to FieldSchema for backward compatibility, deriving them from the unified schema. (sphinx_needs/needs_schema.py)
  • Updated schema creation for extra options and core fields to use deep copies of the schema and to handle array types correctly. (sphinx_needs/needs.py) [1] [2]

Error handling improvements:

  • Changed error handling during schema creation for core fields, extra options, and extra links to raise NeedsConfigException immediately on any error, instead of logging and continuing. This ensures misconfigurations are caught early and clearly reported. (sphinx_needs/needs.py, tests/__snapshots__/test_api_usage.ambr) [1] [2] [3] [4]

Code cleanup and consistency:

  • Removed the now-obsolete validate_extra_option_schemas function and related logic, as schema validation is now handled directly in the schema creation process. (sphinx_needs/schema/config_utils.py) [1] [2] [3]
  • Updated schema processing to use the new schema structure when building validation schemas for extra options. (sphinx_needs/schema/process.py)

Documentation:

  • Added ExtraOptionSchemaTypes to the list of documented classes in docs/conf.py.

@chrisjsewell chrisjsewell changed the title 🔧 Store schema on FieldSchema 🔧 Store full schema on FieldSchema Dec 16, 2025
@codecov
Copy link

codecov bot commented Dec 16, 2025

Codecov Report

❌ Patch coverage is 87.87879% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.12%. Comparing base (4e10030) to head (c91d3a2).
⚠️ Report is 215 commits behind head on master.

Files with missing lines Patch % Lines
sphinx_needs/needs.py 66.66% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1603      +/-   ##
==========================================
+ Coverage   86.87%   88.12%   +1.24%     
==========================================
  Files          56       70      +14     
  Lines        6532     9687    +3155     
==========================================
+ Hits         5675     8537    +2862     
- Misses        857     1150     +293     
Flag Coverage Δ
pytests 88.12% <87.87%> (+1.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@chrisjsewell chrisjsewell requested a review from Copilot December 16, 2025 16:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

This will make it easier to implement #1547
Copy link
Member

@ubmarco ubmarco left a comment

Choose a reason for hiding this comment

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

LGTM

@chrisjsewell chrisjsewell merged commit dd0fb6b into master Dec 17, 2025
23 checks passed
@chrisjsewell chrisjsewell deleted the schema-on-field branch December 17, 2025 02:11
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