Skip to content

generate run-length-encoding #288

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 3 commits into from
Apr 15, 2016
Merged

generate run-length-encoding #288

merged 3 commits into from
Apr 15, 2016

Conversation

Cohen-Carlisle
Copy link
Member

TODO: implement #decode in example.rb
find place in config.json
why isn't execution bit set on generated tests?

@kotp
Copy link
Member

kotp commented Apr 11, 2016

Try the tool in bin to set the executable bit. Let me know if it doesn't do the right thing.

@Cohen-Carlisle
Copy link
Member Author

implement #decode in example.rb

Working on this.

find place in config.json

Any thoughts on this?

why isn't execution bit set on generated tests?

Addressed by #289

@Cohen-Carlisle
Copy link
Member Author

Something seems incorrectly configured... had a similar issue on my pre-push checks... they are not finding example.rb.

Other than that, any thoughts on where this test should live in the track/config.json?

@Cohen-Carlisle Cohen-Carlisle changed the title (wip) generate run-length-encoding generate run-length-encoding Apr 13, 2016
@kotp
Copy link
Member

kotp commented Apr 13, 2016

You are missing the last part of the file name you are wanting to require... require_relative 'run_length_encoding' should fix that problem.

@Cohen-Carlisle
Copy link
Member Author

Updated.
Still wondering where a good place for this exercise is. I placed it where I thought it would fit, but it was kind of a wild guess 😆
If anyone has any thoughts let me know, I'll change it if need be and then squash.

@kotp
Copy link
Member

kotp commented Apr 14, 2016

Generally speaking 'do' is one of the (around 42) reserved bare words in Ruby. Should we be in the habit of using it?

@Cohen-Carlisle
Copy link
Member Author

Yeah, that is probably a bad idea. do blows up without an explicit receiver.
I used it to make this line <= 80 chars, as encode and decode were too big and splitting it across multiple lines would require some reworking of the generator files and I'm lazy 😞

@kotp
Copy link
Member

kotp commented Apr 14, 2016

The 80 is a guide, of course. I also am not sure which line, your link is not helpful as for some reason it did not highlight the line as I thought it would.

I noticed there are other files that define a method do. Hamming is used as the example currently for those that want to work on creating the generators. I think it is the wrong thing to do (no pun intended.). Until I understand the generators a little bit better, though, I have no alternative suggestion.

@kotp
Copy link
Member

kotp commented Apr 14, 2016

Now that I have a little better understanding, I am using 'actual' in place of 'do'.

@kytrinyx I think that is a better representation of what we want to communicate. It may also help with the stroop effect of being in ERB at that level.

@kotp
Copy link
Member

kotp commented Apr 14, 2016

And now that it is all together, I am using something else. :) But I did avoid 'do' as a method name. And made the erb simpler to understand as well.

@Cohen-Carlisle
Copy link
Member Author

More and hopefully final changes.

@kotp kotp merged commit d962df9 into exercism:master Apr 15, 2016
@kotp
Copy link
Member

kotp commented Apr 15, 2016

Looks good to me, version numbers are right, reads well, etc. Thanks @Cohen-Carlisle!

@kotp kotp removed the ready label Apr 15, 2016
@Cohen-Carlisle Cohen-Carlisle deleted the generate-run-length-encoding branch April 15, 2016 04:16
kotp added a commit that referenced this pull request Apr 16, 2016
gchan pushed a commit to gchan/xruby that referenced this pull request Oct 18, 2016
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.

2 participants