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

grep: Make exercise schema-compliant #712

Merged
merged 3 commits into from
Mar 13, 2017
Merged

grep: Make exercise schema-compliant #712

merged 3 commits into from
Mar 13, 2017

Conversation

rbasso
Copy link
Contributor

@rbasso rbasso commented Mar 12, 2017

This exercise has 3 text files encoded in the canonical-data.json, which is incompatible with the new JSON schema.

We are discussing in #699 what is the best way to deal with sharing test data among test cases, and it seems there isn't much support for adding this feature to the schema.

Anyway, that discussion doesn't need to suspend the migration of the current test data.

I proposed here to simply move the encoded files to the comments and proceed with the migration.

Related to #625.

rbasso added 3 commits March 12, 2017 18:20
Move the 'files' object representing the three files that must be
present when running the tests to the comments, because the new
JSON schema doesn't allow shared data among test cases.

This is the only remaining exercise having a shared data structure,
and there are no plans at the moment to add support for this
feature, so are we moving the data to preserve the information and
allow compliance with the new standard.
@rbasso
Copy link
Contributor Author

rbasso commented Mar 12, 2017

Updated to fix some minor formatting issues.

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.

all commits do what they claim to do

I wonder if any language track will complain that it's too hard to extract the files from the comments (find lines surrounded by |, strip trailing newlines), but until someone comes and argues for it I can't really argue on their behalf.

@rbasso
Copy link
Contributor Author

rbasso commented Mar 12, 2017

I wonder if any language track will complain that it's too hard to extract the files from the comments (find lines surrounded by |, strip trailing newlines), but someone comes and argues for it I can't really argue on their behalf.

I was not even considering the possibility of algorithmically extracting the lyrics from the comments, @petertseng. 😄

I think this PR is just an immediate solution for preserving the lyrics and allow compliance. I would really like to see a rewriting of this exercise allowing more automatic test generation, but waiting for it would postpone #625, and I think that it would be good to have #658 merged soon to enforce compliance in Travis-CI.

@petertseng
Copy link
Member

Found out I missed a word:

but someone comes and argues

but until someone comes and argues

@rbasso rbasso merged commit a7faa76 into exercism:master Mar 13, 2017
@rbasso rbasso deleted the grep-schema branch March 13, 2017 01:32
emcoding pushed a commit that referenced this pull request Nov 19, 2018
Change: Test for incorrect greedy algorithm
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.

3 participants