-
-
Notifications
You must be signed in to change notification settings - Fork 554
acronym: Update json for new "input" policy #1032
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
Conversation
"expected": "GIMP" | ||
}, | ||
{ | ||
"description": "hyphenated", | ||
"property": "abbreviate", | ||
"phrase": "Complementary metal-oxide semiconductor", | ||
"input": { | ||
"phrase": "Complementary metal-oxide semiconductor" |
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.
minor knit pick. "phrase" should be indented a couple of spaces on last 4 cases. Otherwise, 👍
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.
👍 Thanks
01df6bd
to
622566e
Compare
@rpottsoh Since approved, can I merge, squash, or rebase it in? Or should I wait until one of the maintainers for problem-specifications does it? |
@Stargator I am fine either way. Others' may want to review this, but likely no one will find fault with the proposed changes in your commit. |
Alright, I'll give it until tomorrow night. I'm in no rush. Thanks. |
"cases": [ | ||
{ | ||
"description": "Abbreviate a phrase", | ||
"cases": [ | ||
{ | ||
"description": "basic", | ||
"property": "abbreviate", | ||
"phrase": "Portable Network Graphics", | ||
"input": { | ||
"phrase": "Portable Network Graphics" |
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.
This may want the same treatment that I have suggested in #1033. Do we really need a "phrase" object since it is the only item with in "input"?
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.
don't care what y'all decide, but #996 (comment) and #996 (comment) are explicitly in favour of having "input": {"phrase": "Portable Network Graphics"}
not "input": "Portable Network Graphics"
therefore that is the only thing I support in #998.
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.
Never mind. I agree with @petertseng.
@Stargator I think you can merge this when ready. |
#996 Updated the schema for canonical data. So I am updating the acronym exercise's json to reflect the change.