Import: document EDA/KiCad columns, add regression tests, fix eda_invisible inversion#1436
Open
Sebbeben wants to merge 1 commit into
Open
Import: document EDA/KiCad columns, add regression tests, fix eda_invisible inversion#1436Sebbeben wants to merge 1 commit into
Sebbeben wants to merge 1 commit into
Conversation
4b38eda to
f25eb26
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1436 +/- ##
============================================
+ Coverage 58.49% 58.50% +0.01%
- Complexity 8701 8703 +2
============================================
Files 638 638
Lines 28123 28125 +2
============================================
+ Hits 16451 16455 +4
+ Misses 11672 11670 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…nvisible inversion - Document the eda_* import columns in docs/usage/import_export.md: the flat form and its short aliases, and the nested eda_info.* form used by the CSV/JSON export and the shipped part_import_example.csv. - Add EntityImporterTest coverage locking in that BOTH the nested eda_info.* form and the flat form import the EDA fields correctly (the nested form already worked via the serializer but was untested, so an export -> re-import round-trip could silently regress). - Fix the eda_invisible alias: it was a plain key-rename to eda_visibility, so eda_invisible=1 stored visibility=true (made the part visible - the opposite of the name). It is now handled explicitly and inverted.
f25eb26 to
16f0c4d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
While importing parts with KiCad/EDA columns I hit some rough edges around the EDA import fields. This PR
does three small things: documents the EDA import columns (they were undocumented), locks in the two
supported column forms with tests, and fixes one alias that stored the wrong value.
1. Document the EDA / KiCad import columns
The
eda_*import columns weren't documented anywhere, which made the flat names hard to discover. Added asection to
docs/usage/import_export.mdlisting them, and noting that the importer accepts both:eda_kicad_symbol,eda_value, … and short aliases likekicad_symbol), andeda_info.*form used by the CSV/JSON export and by the shippedpart_import_example.csv(
eda_info.kicad_symbol, …).2. Regression tests for both column forms
The nested
eda_info.*form already works today (the CSV encoder un-flattens the dotted headers and theeda_infoembeddable is in theimportserialization group), but it wasn't covered by a test — so an export →re-import round-trip, and the EDA columns in the example CSV, could silently regress. Added to
EntityImporterTest:testImportAcceptsNestedEdaInfoColumns—eda_info.kicad_symbol/…footprint/…reference_prefix/…valuefill the part'sEdaInfo.testImportAcceptsFlatEdaColumns— the flat form and its aliases still work.3. Fix:
eda_invisiblestored the inverse valueeda_invisiblewas mapped toeda_visibilityby a plain key-rename, soeda_invisible=1setvisibility=true— i.e. it made the part visible, the opposite of what the name says. It's now handledexplicitly and inverted, so
eda_invisible=1correctly hides the part. Covered bytestImportInvisibleAliasInvertsVisibility.Notes
No schema/behaviour change beyond the
eda_invisiblefix; the nested-column support was already present and isonly being documented and tested here.