Closed depfu[bot] closed 4 days ago
@happy5214 Just a note that hed-validator 3.15.5 broke the legacy validator. It appears (#2169) to work fine with the schema validator.
I just want to check whether this is intentional, since it was a patch release instead of a minor release.
If the intent going forward is to stop supporting the legacy validator (which is a totally reasonable position to take), it would be good to know, and we'll freeze that dependency. Our hope is to transition the world away from the legacy validator in the coming months, in any case.
@happy5214 Just a note that hed-validator 3.15.5 broke the legacy validator. It appears (#2169) to work fine with the schema validator.
I just want to check whether this is intentional, since it was a patch release instead of a minor release.
If the intent going forward is to stop supporting the legacy validator (which is a totally reasonable position to take), it would be good to know, and we'll freeze that dependency. Our hope is to transition the world away from the legacy validator in the coming months, in any case.
No, that was not the intent. It's actually a bug, but in this case, hed-validator is right and both the hed.js
and hed.spec.js
modules in the legacy validator are wrong.
Long story: The tests have improperly nested HED data in three sidecars (the HED
level is the third level, where it should be the second level). This is causing https://github.com/hed-standard/hed-javascript/blob/916236a58b9912cb4fefc0e79aa9c28d0f35cedb/bids/types/json.js#L107 to (correctly) throw an IssueError
for what amounts to a SIDECAR_INVALID
HED error. However, the try
block starting at https://github.com/bids-standard/bids-validator/blob/5646d9dee39eb9228881ec7e57887021cf3072b0/bids-validator/validators/hed.js#L34 incorrectly assumes all caught errors are missing schema errors, leading to improper issue reporting.
The tests need to be fixed to correct the level of HED
in the three bad sidecars, and hed.js
needs to be re-coded to find another way to deal with a null
return from the validation function.
@depfu refresh
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 85.70%. Comparing base (
9d0d585
) to head (eaf1f86
). Report is 2 commits behind head on master.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This is your weekly update of all npm dependencies. Please take a good look at what changed and the test results before merging this pull request.
What changed?
↗️ @next/env (indirect, 14.2.13 → 14.2.15, patch) · Repo · Release
✳️ @next/eslint-plugin-next (14.2.13 → 14.2.15, patch)
✳️ chai (5.1.1 → 5.1.2, patch) · Repo · Changelog · Release · Diff
✳️ eslint-config-next (14.2.13 → 14.2.15, patch)
✳️ hed-validator (3.15.4 → 3.15.5, patch) · Repo · Release · Diff
✳️ next (14.2.13 → 14.2.15, patch) · Repo · Release
Depfu will only send you the next scheduled PR once you merge or close this one.
All Depfu comment commands