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

Add the correct command to run the tests in README.md #130

Merged
merged 1 commit into from
Jan 7, 2018

Conversation

budmc29
Copy link
Member

@budmc29 budmc29 commented Jan 4, 2018

No description provided.

Copy link
Contributor

@sjwarner-bp sjwarner-bp left a comment

Choose a reason for hiding this comment

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

I am confused as to why there are two PRs (this and #131 ).

They look to contain some of the same changes, and generally look related.

Furthermore, I think the reason this happens is because our exercise readme insert chooses to say bats whatever_test.sh.

If we are going to change this, it should probably be changed in the exercise readme insert.

Any suggestions?

@budmc29
Copy link
Member Author

budmc29 commented Jan 4, 2018

Don't worry, you don't need to write the same message in multiple places, I am subscribed to receive notifications for this track.

They seem related, but are different. This PR updates the README.md files to show the correct bats command, and #131 renames the plural version of the commands and the tests names themselves.
For example this introduces bats anagram_tests.sh and #131 changes it to bats anagram_test.sh (observe the missing 's').

As for bats whatever_test.sh being inserted, I don't know if we can make it somehow to pick up the correct name, so it's fine as it is, we'll just have to keep an eye for that when integrating PR's with exercises.

@sjwarner-bp
Copy link
Contributor

They seem related, but are different. This PR updates the README.md files to show the correct bats command, and #131 renames the plural version of the commands and the tests names themselves.

Thanks for explaining.

As for bats whatever_test.sh being inserted, I don't know if we can make it somehow to pick up the correct name.

When using configlet to generate READMEs (as we should), it will just copy the contents of our insert. As such, I don't think there is a way to get the slug name (and I don't think it's too necessary, as it's probably obvious to users that whatever_test.sh doesn't exist).

Maybe the improvement we need to make here is to indicate whatever is just some placeholder - maybe <slug_name>_test.sh? What do you think?

@budmc29
Copy link
Member Author

budmc29 commented Jan 7, 2018

Maybe the improvement we need to make here is to indicate whatever is just some placeholder - maybe <slug_name>_test.sh? What do you think?

Yes, that sounds like something that would make it harder to forged to update and can be caught easier by the implementer and the reviewer.

@budmc29 budmc29 merged commit 9d5645b into exercism:master Jan 7, 2018
This was referenced Jan 7, 2018
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