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

make the tick and cross symbols optional #14

Merged
merged 1 commit into from
Aug 12, 2014
Merged

make the tick and cross symbols optional #14

merged 1 commit into from
Aug 12, 2014

Conversation

stocksr
Copy link
Contributor

@stocksr stocksr commented Aug 11, 2014

make the tick and cross symbols optional
They don't render correctly on windows ;-(

@bcaudan
Copy link
Owner

bcaudan commented Aug 11, 2014

I understand the issue but I have some comments:

  • You can run the tests locally or is there some issue on windows with those too?
  • It could be better to allow customization of prefixes instead of suppressing them. Like the option for colors:
prefixes: {
  success: '✓',
  failure: '✗',
  skipped: '-'
}
  • Would you mind to squash your commits?

@stocksr
Copy link
Contributor Author

stocksr commented Aug 12, 2014

hi @bcaudan, thanks for the feedback.
I have made some changes based on your comments

Running the tests on windows fails with

C:\Users\Robert Stocks\Documents\jasmine-spec-reporter>npm test

> [email protected] test C:\Users\Robert Stocks\Documents\jasmine-spec
-reporter
> ./node_modules/jasmine-node/bin/jasmine-node --coffee --noStack spec

'.' is not recognized as an internal or external command,
operable program or batch file.
npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0

Also I am not sure how to squash commits.

@bcaudan
Copy link
Owner

bcaudan commented Aug 12, 2014

LGTM except that the tests are still failing...
About running the tests on windows, can you try to launch this command from the project directory:

node_modules/jasmine-node/bin/jasmine-node --coffee --noStack spec

About squashing the commits, the idea is to have a single commit for your PR.
You can have a look here to see how it works.

@stocksr
Copy link
Contributor Author

stocksr commented Aug 12, 2014

to run the test on windows I had to run

node node_modules\jasmine-node\bin\jasmine-node --coffee --noStack spec

all the tests are now passing

About to attempt the commit squash

They don't render correctly on windows ;-(
bcaudan added a commit that referenced this pull request Aug 12, 2014
make the tick and cross symbols optional
@bcaudan bcaudan merged commit 004dcf1 into bcaudan:master Aug 12, 2014
@bcaudan
Copy link
Owner

bcaudan commented Aug 12, 2014

Thanks a lot, it is available in 0.6.0!

@stocksr stocksr deleted the patch-1 branch August 13, 2014 06:10
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