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.
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
Structured validation results #1104
Structured validation results #1104
Changes from all commits
696e7fe
037dc5e
3bc01ba
43ab1f5
1f88e4e
8e8e4b7
8561215
f082382
dd77af2
aefb337
1c8ddd3
cb6d494
e2b21ed
8af720a
ea4e790
db175a2
a90c99a
3304b45
2a9377e
6a0875b
4f248b9
a564d0d
cc57bfc
15e1cfb
89a070a
fe238b6
25194ee
e630fb8
72f47a0
b51e92e
94a1793
ec59dad
43e17e9
4f2e4ae
bf5c0f1
697b24d
bb52d56
ee9492a
c828628
2f1c6a7
155e557
f30ad99
676cbc3
9a8861b
74cc797
db6a81f
715f70e
fb5847c
e0027c3
453cd65
786729a
d0070bd
b205aaf
d04f4bd
7d21f4e
4ba4b7b
23e54be
28e7374
3702eac
2d9723c
61e5d3a
1711354
122581a
e25375b
e292718
c4a36ca
cf34de3
e363670
6a56acd
b4ad1bc
de27873
6b7e680
ccf9491
b3106f4
49c9a0a
9861be1
f387998
04ee73c
d161b46
482246a
1691547
889d2e7
dd03eb5
b2f4c2e
0d77a0d
e263bf4
8b58dce
f2f577d
5c1890c
e30809d
87048c7
d4b35b4
acd78b7
cc2ad26
b5e16b1
9f49cc9
382c3ae
8d71483
2d4e284
2c8d034
80db60e
af48872
15ce3c7
8c430f6
6348f64
ed5f353
65ae2c9
b4d5dcd
624e3b0
41df99d
09e526b
e38577b
456fafd
857d7b8
56973d3
4eb0c70
404968b
6ac5b99
abc8e8d
f80b5b4
011b45d
76ddf06
2c7dd6a
6205109
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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.
what meaning of `purview` name here? the ones from dict not sure if match.
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.
why not to make display_errors just to get a list of them instead of breaking them apart like this just to
zip
them back together inside that function?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.
https://www.merriam-webster.com/dictionary/purview
1b or 2. it's basically a synonym of scope since scope was taken.
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.
Well, the idea was to have a separate function which does as little as possible other than
click.secho()
. The lists need to be populated differently depending on the grouping anyway, and additional edge cases will pop up as we have e.g. multi-path errors, so we do all that in the parent function and just echo depending on the list lengths. If you want I can roll it all up in a single function.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 still feel that it is odd design here, not ready ATM to give a complete solution, could at least be
display_errors(purviews, issues)
or alike, with all needed unzipping/zipping done inside if needed. but ok, we could RF laterThere 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'd really suggest leaving it like this for now, and see if moving that code bit to another function is still needed when we merge validation functions.