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

Remove Bluebird.longStackTraces() #1496

Merged
merged 3 commits into from Sep 2, 2017
Merged

Remove Bluebird.longStackTraces() #1496

merged 3 commits into from Sep 2, 2017

Conversation

anshulwadhawan
Copy link
Contributor

@anshulwadhawan anshulwadhawan commented Aug 26, 2017

Fixes #1492.

@anshulwadhawan
Copy link
Contributor Author

Remove Bluebird.longStackTraces() issue (#1492) solved

@anshulwadhawan
Copy link
Contributor Author

Can someone help me with these 2 tests i.e. the Travis CI build and AppVeyor build that take place.
What all do these tests depend on ?

@forresst
Copy link
Contributor

Travis CI and AppVeyor do not launch exactly the same scripts.

Travis CI runs npm run test (default): "test": "xo && flow check test/flow-types && tsc -p test/ts-types && nyc tap --no-cov --timeout=300 --jobs=4 test/*.js test/reporters/*.js"

AppVeyor runs npm run test-win: "test-win": "tap --no-cov --reporter=classic --timeout=300 --jobs=4 test/*.js test/reporters/*.js"

The Travis CI script performs additional tasks (xo, flow and tsc): It is not useful to run them on both as they will do the same job of several syntax checking and good practice.

I advise you to read how to do a Pull Request: to summarize, on your local repository, you have to do an npm install and after your modifications, run npm test. This will launch the same tests as Travis CI.

Finally, the errors you see on Travis CI is XO asking you to change the order of 'import'.

Hopefully my answer will help.

I'm not sure why removing the Bluebird import suddenly causes this
linter error, but it should be fine to move up these two imports.
@novemberborn novemberborn changed the title Solved issue #1492 Remove Bluebird.longStackTraces() Aug 27, 2017
@novemberborn
Copy link
Member

Thanks for your PR @anshulwadhawan!

I'm not sure why linting started to fail just because the Bluebird import was removed. I've pushed a commit that should solve that.

From the issue:

As long as we process the require modules early enough, users can still enable long stack traces if they use Bluebird in their own code. It would be good to have an integration test for this though.

Would you like to give this a go? We can leave it as a follow-up so no worries if not.

@anshulwadhawan
Copy link
Contributor Author

anshulwadhawan commented Aug 28, 2017

I am new to Open Source.Can you guys help me with some of my queries:
Is my PR merged ? or you want me to use your commit.
Please help me get my first PR merged !
@novemberborn @forresst

@novemberborn
Copy link
Member

Hey @anshulwadhawan,

Is my PR merged ?

Not yet, but I'll do so soon.

or you want me to use your commit.

I've pushed it to your branch, so it's already included in this PR.

@novemberborn novemberborn merged commit ebf78b3 into avajs:master Sep 2, 2017
@novemberborn
Copy link
Member

Thank you @anshulwadhawan!

mliou8 pushed a commit to mliou8/ava that referenced this pull request Sep 11, 2017
kevva pushed a commit that referenced this pull request Sep 13, 2017
@mliou8 mliou8 mentioned this pull request Sep 17, 2017
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