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

postuninstall hook feature #2030

Closed
wants to merge 6 commits into from
Closed

postuninstall hook feature #2030

wants to merge 6 commits into from

Conversation

samjacobclift
Copy link

postuninstall hook for bower

See the original issue here #1451

Any and all feedback welcome!

@pertrai1
Copy link
Contributor

@royka Can you check the failing build for your test?

@samjacobclift
Copy link
Author

@pertrai1 Taken a look but I can reproduce the failing build, I've tried running the test suite with different node versions but can't see why the test fails in Travis

@faceleg
Copy link
Member

faceleg commented Nov 22, 2015

@royka did you run it with node 4.2?

@samjacobclift
Copy link
Author

ran it on node version 4.2.2 on OS X as well as 5.0. Just run grunt test right?

@samjacobclift
Copy link
Author

just ran the tests on ubuntu:12.04 image with node version 4.2 test still passed

@samjacobclift
Copy link
Author

also ran on windows 10 with node 5.0, maybe I'm missing something?

@sheerun
Copy link
Contributor

sheerun commented Nov 25, 2015

Maybe it's some kind of race condition? ..

@sheerun
Copy link
Contributor

sheerun commented Nov 27, 2015

@royka You need to wrap scripts.postuninstall in another function, just like here: https://github.com/royka/bower/blob/postuninstall-hook-feature/lib/core/Project.js#L109

That's how promises work.

Please also rebase on current master branch and force push changes. Thank you!

@sheerun
Copy link
Contributor

sheerun commented Nov 27, 2015

Also, please add another test instead of modifying existing one :)

@samjacobclift
Copy link
Author

Thanks @sheerun will update with the changes!

@samjacobclift
Copy link
Author

Pushed the changes suggested test pass locally so waiting for CI to come back but all feedback is welcome, wanted and very much needed :)

@sheerun
Copy link
Contributor

sheerun commented Dec 2, 2015

Unfortunately some Travis builds are still failing :) Also, could you squash all commits when they pass?

@samjacobclift
Copy link
Author

Will get to work on this soon, sorry to leave it hanging!

@sheerun
Copy link
Contributor

sheerun commented Apr 1, 2016

Hey. Any progress?

@sheerun
Copy link
Contributor

sheerun commented Apr 14, 2016

Commited in #2252

@sheerun sheerun closed this Apr 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants