JacobNinja / exercism-analysis

2 stars 2 forks source link

Ruby: Shebang rule for tests #15

Closed budmc29 closed 7 years ago

budmc29 commented 7 years ago

Right now when you submit your code file example.rb and you choose to also submit your test file, example_test.rb, Rikki will pick on that shebang.

Skipping that check for test files I believe is a good behaviour. @kotp mentioned that there is a low-priority task for that. I'd like to give this a try if no one else is working on it and if it's worth while.

What do you think? Cheers

@kytrinyx @JacobNinja

kotp commented 7 years ago

The solution analysis is maybe flawed. It is probably better to leave it in the analysis, but to remove the shebang from the test files on the exercism/ruby repository. This can be done via the generators for those files that are generated, and may be done individually on those tests in the exercises folder. See https://github.com/exercism/ruby

kytrinyx commented 7 years ago

I think the analyzer is correct, but we can decide if we want to work around it either in https://github.com/exercism/rikki-ruby-analyzer or in rikki itself to not complain about test files.

budmc29 commented 7 years ago

I'd like to give a try for https://github.com/exercism/rikki-ruby-analyzer since I don't know Go. Does that sound good?

kotp commented 7 years ago

Definitely sounds good to do some work in the rikki ruby analyzer, but I don't think the exception for the tests is the right thing. The analyzer is correct. We should not work around it, but remove the shebangs in the test when they are generated (or directly if they are not generated).

That is why I indicated that the analyzer is correct, the solution analysis is probably flawed.

There is a lot that we can do with Rikki though, so that thought, @budmc29, is a good one.

budmc29 commented 7 years ago

I have created a PR: https://github.com/exercism/rikki-ruby-analyzer/pull/6.

Insti commented 7 years ago

I don't think the exception for the tests is the right thing.

I agree with this.

kytrinyx commented 7 years ago

Per the discussion in exercism/ruby#295, we've decided to solve this by removing the shebang in the ruby tests. Closing this.