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

Move donation message to scripts #804

Merged
merged 5 commits into from
Aug 24, 2017
Merged

Conversation

andrejsm
Copy link
Contributor

@andrejsm andrejsm commented Aug 10, 2017

This fixes the issue where installation would fail in environments where npm is not available.

Fixes #803

this fixes issue where installation would fail in environments where
`npm` is not available.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 50d4c69 on andrejsm:master into e026d9e on developit:master.

Copy link
Member

@developit developit left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a bunch for the PR. Any idea if this works under Windows? If not I can check.

@andrejsm
Copy link
Contributor Author

Please do check if it works under Windows

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d8b0199 on andrejsm:master into ** on developit:master**.

@developit
Copy link
Member

Seems like it doesn't work under Win.

@andrejsm
Copy link
Contributor Author

Thanks for checking. I'll then rewrite it to node script. Will get to this within a couple of hours.

@andrejsm
Copy link
Contributor Author

Moved from Shell script to Nodejs script. Works on Windows as well.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 846aa59 on andrejsm:master into 68fa510 on developit:master.

@developit
Copy link
Member

Awesome - tested that out locally too and it works well. Only thing I'd like to see (and I don't mind doing it myself post-merge) is instead of using ./scripts, put the script in the existing config/ dir.

@andrejsm
Copy link
Contributor Author

Moved to config/

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling fe88ebf on andrejsm:master into 68fa510 on developit:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 8a98477 on andrejsm:master into 0dea3b7 on developit:master.

@developit developit merged commit 4ea1bc7 into preactjs:master Aug 24, 2017
@developit
Copy link
Member

Works great!

@cletusw cletusw mentioned this pull request Aug 24, 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.

Can't install when npm is not available
3 participants