Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report warning instead of error if only warnings are returned by hed-validator #1158

Merged
merged 5 commits into from
Feb 2, 2021

Conversation

happy5214
Copy link
Collaborator

The existing implementation errors out even if only warnings are issued
by hed-validator. This is not desirable, and has been fixed here. A
new issue code has been added, and a new test case has also been added,
though it will not be correct until another change is made upstream.

This is of relatively high severity, as it prevents OpenNeuro from properly validating
any BIDS datasets with HED tag extensions in the sidecar, which generate warnings.

happy5214 and others added 5 commits January 29, 2021 05:14
…validator

The existing implementation errors out even if only warnings are issued
by hed-validator. This is not desirable, and has been fixed here. A
new issue code has been added, and a new test case has also been added,
though it will not be correct until another change is made upstream.
@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #1158 (b6dc653) into master (15d9dc5) will increase coverage by 0.02%.
The diff coverage is 88.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1158      +/-   ##
==========================================
+ Coverage   79.21%   79.23%   +0.02%     
==========================================
  Files          78       78              
  Lines        2593     2601       +8     
  Branches      592      594       +2     
==========================================
+ Hits         2054     2061       +7     
  Misses        400      400              
- Partials      139      140       +1     
Impacted Files Coverage Δ
bids-validator/validators/events/hed.js 92.66% <88.23%> (-0.41%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15d9dc5...b6dc653. Read the comment docs.

@rwblair rwblair merged commit 227b1f1 into bids-standard:master Feb 2, 2021
@rwblair
Copy link
Member

rwblair commented Feb 2, 2021

@happy5214 can you double check my work on the HED issue numbers, due to recent merge I moved them down, wanted to make sure the issue codes in the tests match the correct error messages. Might of been hasty with the merge.

@happy5214
Copy link
Collaborator Author

@happy5214 can you double check my work on the HED issue numbers, due to recent merge I moved them down, wanted to make sure the issue codes in the tests match the correct error messages. Might of been hasty with the merge.

@rwblair Sorry for the late reply. Issue code 209 and 210 were indeed flipped (the error was before the warning in the original, and the issue codes in the actual code reflect that), so that needs to be fixed.

happy5214 added a commit to happy5214/bids-validator that referenced this pull request Feb 5, 2021
The sidecar warning and error codes were inadvertently flipped in
the process of writing bids-standard#1158. This rectifies this error.
rwblair added a commit that referenced this pull request Feb 9, 2021
rob-luke pushed a commit to rob-luke/bids-validator that referenced this pull request Jan 31, 2022
The sidecar warning and error codes were inadvertently flipped in
the process of writing bids-standard#1158. This rectifies this error.
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.

2 participants