Skip to content

Conversation

rmagrin
Copy link
Contributor

@rmagrin rmagrin commented Nov 16, 2022

The jasmine and mocha reporters were not being included in the npm package.

@mariovisic
Copy link
Contributor

Looks good, but you also commited some changes to util/tracer.test.js as well ?

@mariovisic
Copy link
Contributor

Thanks for picking this up 👍

@rmagrin
Copy link
Contributor Author

rmagrin commented Nov 17, 2022

Looks good, but you also commited some changes to util/tracer.test.js as well ?

There was some flakiness in the tests that made the first execution fail. I just changed the util/tracer.test.js to see if it fixed the flakiness issue.

I can remove the test changes if needed.

Here is the test execution that failed: https://buildkite.com/buildkite/javascript-test-collectors/builds/112#01848200-5eed-4e53-8b42-53d7678b3a8f

@rmagrin
Copy link
Contributor Author

rmagrin commented Jan 10, 2023

@mariovisic do you have any more comments here?

@mariovisic
Copy link
Contributor

no sorry, i'm not the maintainer, so you'll need to have someone from Buildkite review this. Though just looking you still have made changes to util/tracer.test.js which are unrelated, so maybe split these into another PR to make it easier to review, or add some rationale to your description in the PR about these changes.

@mariovisic
Copy link
Contributor

oh sorry, missed the comment above about your change, makes sense, but yeah, you'll need to get someone from Buildkite to review 👍

@swebb
Copy link
Contributor

swebb commented Feb 2, 2023

Hi @rmagrin. Sorry for the delay. Thank you for the contribution. I'll raise a ticket for this and we'll see if we can get it merged.

Copy link
Contributor

@KatieWright26 KatieWright26 left a comment

Choose a reason for hiding this comment

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

Hi @rmagrin, thanks for your contribution! I'm going to merge your PR now, and your changes will go out in a release shortly.

@KatieWright26 KatieWright26 merged commit 0834a2e into buildkite:main Feb 7, 2023
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.

4 participants