-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
reporter-specific options and support for the xunit reporter to output to a file (#2) #1218
Conversation
@travisjeffery I believe I've addressed all your comments from the other PR. Are there any other concerns? If not can you please merge this, and I'd also appreciate if you could push a new release that contains the change. |
Code looks straight forward to me, FWIW. I also happen to be using it on my CI setup, so I'm somewhat biased in wanting to see it upstream so I don't have to maintain a fork. |
I've waited for this. Please merge it. |
@travisjeffery, @visionmedia If for some reason you don't want to take this whole change, how about just taking commit 6acbc7f which adds the |
This with reporter-specific options would be great - it is must. I also plan to develop |
Rebased on top of the latest master to fix merge conflicts. Any updates from the maintainers as to what if anything is holding up merge? @travisjeffery I thought I addressed all of your concerns back in May. |
@demmer the new maintainers are just cautiously working their way through the issues and pull requests. Thanks for the rebase and the bump - I'll look at this shortly 😄 |
ya i have a branch where i made some changes, lemme see if i can dig that up and merge it |
Thanks! On Sat, Jul 12, 2014 at 1:51 PM, Travis Jeffery [email protected]
Michael Demmer |
It's been months since this pull request was submitted. |
👍 |
2 similar comments
👍 |
👍 |
@travisjeffery Another couple of months have gone by so I'd like to bump this once again. Again is there anything that is holding up this merge? You mentioned that you had "made some changes". |
Looking forward to seeing this integrated. Had to remove all logging in my code so I could generate valid XUnit output with mocha. |
👍 |
Any update on this? Would love to see this merged in and made available. |
👍 |
👍 This is a basic necessity for our CI. Without this, I can run my tests on my own and tell the rest of the world I'm happy with my code but I can't convince our pipeline this is the case. @demmer 's put the work in of plugging this in a couple of times and has rebased to deal with the wait. Travis CI passes! Can't somebody just click 'accept'? |
Mm yep.. Months waiting.. |
Seriously, I'm at this point 😢 |
@travisjeffery if you have a branch w your changes somewhere I can take care of this. |
You should be able to merge this pull request if I'm not mistaken. https://help.github.com/articles/merging-a-pull-request/ |
35c1580
to
64dfc0b
Compare
reporter-specific options and support for the xunit reporter to output to a file (#2)
Catching up with mochajs/mocha@30582e64 (if a reporter has a .done method, call it before exiting, 2014-05-17, mochajs/mocha#1218).
Catching up with mochajs/mocha@30582e64 (if a reporter has a .done method, call it before exiting, 2014-05-17, mochajs/mocha#1218).
(Rework of #1106 to merge cleanly onto master and to address PR comments.)
Support for generic reporter options and an optional .done() hook for reporters
to clean up before exiting.
The xunit reporter uses these hooks to optionally write output to a file when run
in node. By default it still uses console.log to keep support for running in browser.