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

implement exercise nucleotide-count #80

Merged
merged 5 commits into from
Sep 5, 2017
Merged

implement exercise nucleotide-count #80

merged 5 commits into from
Sep 5, 2017

Conversation

Smarticles101
Copy link
Member

I did my best to choose topics and a difficulty for the exercise. The only last two things are should it be a core exercise? If not, what should it be unlocked by?

@kotp
Copy link
Member

kotp commented Sep 3, 2017

This looks good to me. I did notice that there are no tests that are skipped. @exercism/bash are we skipping all but the first test by default to reduce the noise when solving these, until we pass each test?

@Smarticles101
Copy link
Member Author

@kotp It doesn't look like any of the other exercises do this, but it might be a good policy to adapt.

@kotp
Copy link
Member

kotp commented Sep 3, 2017

For the Ruby and Delphi tracks we skip all but the first test. The very first exercise, I think for both tracks, have comments with an explanation of what to do to unskip them, and perhaps also duplicated in the Readme.

This could definitely be a different pull request, that could touch the existing exercises.

@Smarticles101
Copy link
Member Author

Smarticles101 commented Sep 4, 2017

Yeah, that's what we do in the java track too. I'll open another issue for that and start working on it. In the meantime, should we decided on core exercise/unlocked by?

It looks like right now all the exercises depend on panagram. Should I do the same here?

@kotp
Copy link
Member

kotp commented Sep 4, 2017

Leap, Bob, and a couple others are not unlocked by Pangram. But this being unlocked by Pangram seems to be OK, at least at first thought.


Run the tests with:

bats whatever_test.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

replace with bats nucleotide_count_test.sh ?

Copy link
Member Author

@Smarticles101 Smarticles101 Sep 5, 2017

Choose a reason for hiding this comment

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

The readme file for every exercise is generated by configlet. This is the track-specific insert that we have for this track. EXERCISE_README_INSERT


for i in $(seq 1 ${#input}); do
char=${input:i-1:1}
if [ $char = "A" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

shellcheck says:
"Double quote to prevent globbing and word splitting."
--> replace $char by "$char"

Also, to have a better standard in the example, replace $char with ${char} ?
See this for ex.

So $char -> "${char}"

@kenden
Copy link
Contributor

kenden commented Sep 5, 2017

Thanks for the PR! LGTM

@Smarticles101 Smarticles101 merged commit 52476ac into exercism:master Sep 5, 2017
@Smarticles101 Smarticles101 deleted the implement-nucleotide-count branch September 5, 2017 13:31
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.

None yet

3 participants