-
Notifications
You must be signed in to change notification settings - Fork 11
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
OCDS - Group validation errors by type #1168
Conversation
Here's a test file: ocds-data-not-grouped.json.zip Which produces: |
(Closed by accident!) Haven't run I18N process yet, want to get feedback from OCDS people first. |
(pr build fails, push build passes. This is due to having not done I18N strings yet - fine for now) |
f187397
to
72138d0
Compare
Changes made @duncandewhurst |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested with this file and some of the grouping is wrong, examples:
- some format errors are reported in the required fields group:
- Some codelist errors are reported in the format group:
- The errors reported in the 'Other' group are actually required fields errors:
72138d0
to
b853082
Compare
Just to be totally clear @duncandewhurst where would you expect codelist errors to be? |
@odscjames I think they should just appear in the codelist validation errors section but with the error count and location of first 3 errors which are currently missing from that section (see https://github.com/OpenDataServices/cove/issues/1044) |
7f27166
to
a63ae8f
Compare
I have updated http://1159-ocds-group-validation.dev.cove.opendataservices.coop/review/ - @duncandewhurst can you have a look and for the test file you used before, let me know which ones are still out of place? (I'm sure there will be some - sorry) Meanwhile I'm working on how to build a proper list of every possible type, which is also good for me to look through the library we use and learn more about it. |
(Notes for me: https://github.com/Julian/jsonschema/blob/master/jsonschema/validators.py#L473 here is a list of types, but we also have to include whatever we extend it with in .ve/src/libcove/libcove/lib/common.py and also note |
thanks @odscjames I checked and the errors in the 'required fields' and 'format' sections are correct, but there are still some errors under the 'other' heading which seem like they should be under 'format' |
Also the currency codelist error is still duplicated under both 'structural errors - other' and 'codelist errors' |
a63ae8f
to
365c991
Compare
@duncandewhurst Is it ok if we deal with the codelist issue in #1044 ? It will involve looking at different areas of code, was already a problem, and in the interests of keeping pull requests short and thus easier and thus more likely to actually be merged to master, that should probably be a separate pull request. |
Yep no worries, I thought it might be an easy fix whilst looking at
grouping but no worries if not
…On Wed, 24 Apr 2019, 12:26 James, ***@***.***> wrote:
@duncandewhurst <https://github.com/duncandewhurst> Is it ok if we deal
with the codelist issue in #1044
<https://github.com/OpenDataServices/cove/issues/1044> ? It will involve
looking at different areas of code, was already a problem, and in the
interests of keeping pull requests short and thus easier and thus more
likely to actually be merged to master, that should probably be a separate
pull request.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1168 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AES7SFSWZNZJBPCOMYVOK4DPSA7XTANCNFSM4HHWVHUA>
.
|
Thanks. I have updated http://1159-ocds-group-validation.dev.cove.opendataservices.coop/review - do you want to have a look? |
thanks @odscjames I think the only remaining inconsistency is that one "X is too short" error appears in under the "format" group whilst another "X is too short" error appears under the "other" group. I think they should both appear under the "other" group: |
365c991
to
1c24948
Compare
Ready for review |
https://github.com/OpenDataServices/cove/issues/1159