Skip to content

fix: update hole_diameter default value to 0.3mm in pcb_via#362

Open
rushabhcodes wants to merge 3 commits intotscircuit:mainfrom
rushabhcodes:fix-via-defaults
Open

fix: update hole_diameter default value to 0.3mm in pcb_via#362
rushabhcodes wants to merge 3 commits intotscircuit:mainfrom
rushabhcodes:fix-via-defaults

Conversation

@rushabhcodes
Copy link

@rushabhcodes rushabhcodes commented Nov 26, 2025

This pull request makes a minor update to the default value for the hole_diameter property in the pcb_via schema, increasing it from 0.25mm to 0.3mm for consistency.

Copy link
Member

@techmannih techmannih left a comment

Choose a reason for hiding this comment

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

You also need to make them optional. I think you have added them optional in props

@rushabhcodes
Copy link
Author

@techmannih

outer_diameter: distance.default("0.6mm")

  • The field must exist.
  • If the user does not provide a value, it will automatically be set to "0.6mm".
  • The resulting object will always contain a value for outer_diameter.

outer_diameter: distance.default("0.6mm").optional()

  • The field is optional.
  • If omitted, the value will be undefined (not "0.6mm").
  • The default only applies if the field is present, but its value is undefined or invalid (depending on the library).

@rushabhcodes rushabhcodes moved this to Needs Review in tscircuit contributions Nov 27, 2025
@rushabhcodes rushabhcodes moved this from Needs Review to Changes Requested in tscircuit contributions Nov 27, 2025
@rushabhcodes rushabhcodes moved this from Changes Requested to Needs Review in tscircuit contributions Nov 27, 2025
Copy link
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

the defaults really shouldn't be inside circuit-json at all, we should remove the defaults

@rushabhcodes
Copy link
Author

Where should i defaults then??

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.

4 participants