Skip to content

allergies: Update json for new "input" policy #1033

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

Merged
merged 1 commit into from
Dec 14, 2017

Conversation

Stargator
Copy link
Contributor

#996 Updated the schema for canonical data. So I am updating the allergies exercise's json to reflect the change.

@Stargator Stargator self-assigned this Dec 11, 2017
"input": {
"score": 2
},
"expected": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the "expected"'s need to be changed or just the the addition of "input"? I don't know that the present "expected" entries violate the schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#996 Just mentions a change to input, I think expected is always one variable.

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

version?

I expected expected not change, not even for reformatting. Reformatting expected deserves to be in its own commit, not lumped in with "update to new schema" since reformatting is not an update to the new schema.

@Stargator Stargator force-pushed the input-allergies branch 3 times, most recently from 1fd903d to 7fb5996 Compare December 12, 2017 11:13
@Stargator Stargator changed the title Update Allergies Json with new schema allergies: Update json with new schema Dec 12, 2017
@rpottsoh rpottsoh changed the title allergies: Update json with new schema allergies: Update json for new "input" policy Dec 12, 2017
@@ -15,7 +15,9 @@
{
"description": "no allergies means not allergic",
"property": "allergicTo",
"score": 0,
"input": {
"score": 0
Copy link
Member

@rpottsoh rpottsoh Dec 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does "input" really need "score"? Why not just:

"input": 0,

Of course like changes for other cases. Unless we are really married to "score". Others should perhaps weigh in on this point.

Nevermind. @petertseng is right.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't care what y'all decide, but #996 (comment) and #996 (comment) are explicitly in favour of having "input": {"score": 0} not "input": 0 therefore that is the only thing I support in #998.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that detail.

@rpottsoh
Copy link
Member

@Stargator I think you can go ahead and merge this when ready.

@Stargator Stargator merged commit 85dcb59 into exercism:master Dec 14, 2017
@Stargator Stargator deleted the input-allergies branch December 14, 2017 22:46
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.

4 participants