Skip to content

Check if hook exists and don't reinstall. #1

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

Merged
merged 2 commits into from
Nov 27, 2015

Conversation

hdeshev
Copy link
Contributor

@hdeshev hdeshev commented Nov 26, 2015

Installing a hook checks if the hook script already exists (those should be kept in source control), and does nothing in that case.

Added a dependency to glob, so that we do a fuzzy match and handle cases like renaming a hook script like nativescript-dev-typescript.js to 20-nativescript-dev-typescript.js in order to enforce order between several hooks. Example:
https://github.com/NativeScript/sample-ng-todomvc/tree/master/hooks/before-prepare

Note: the glob dependency is set to * so that we try to use any glob version that the user has installed. I'm not sure if that's the right thing to do here.

@hdeshev
Copy link
Contributor Author

hdeshev commented Nov 26, 2015

ping @tailsu, @teobugslayer

@@ -13,6 +13,7 @@
"url": "https://github.com/NativeScript/nativescript-hook.git"
},
"dependencies": {
"glob": "*",
Copy link
Contributor

Choose a reason for hiding this comment

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

I've read your concern, but in case new version of glob has breaking change, the current code will stop working, so I prefer to set exact version (or at least one starting with ^).

@rosen-vladimirov
Copy link
Contributor

👍 from me after setting version of glob :)

@hdeshev hdeshev force-pushed the hdeshev/check-if-hook-exists branch from 9174b35 to 508e135 Compare November 27, 2015 08:37
Do a fuzzy match, trying to handle cases like renaming a hook like
nativescript-dev-typescript.js to 20-nativescript-dev-typescript.js when
having several hooks executing in a defined order.
@hdeshev
Copy link
Contributor Author

hdeshev commented Nov 27, 2015

@rosen-vladimirov Fixed the glob version problem. I can merge and publish the hook, then open a PR for the nativescript-dev-typescript package to use the new version, right?

@rosen-vladimirov
Copy link
Contributor

yes, you can merge this PR and publish new version. For nativescript-dev-typecript, I'm not sure if it is required to publish new version as nativescript-hook is referred with ^:
https://github.com/NativeScript/nativescript-dev-typescript/blob/master/package.json#L33
You can publish new version of ns hook and try tns install typescript in a new project in order to check which version of nativescript-hook will be installed

@hdeshev
Copy link
Contributor Author

hdeshev commented Nov 27, 2015

Gotcha. Publishing...

hdeshev added a commit that referenced this pull request Nov 27, 2015
Check if hook exists and don't reinstall.
@hdeshev hdeshev merged commit 6782e56 into master Nov 27, 2015
@hdeshev hdeshev deleted the hdeshev/check-if-hook-exists branch November 27, 2015 10:30
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.

2 participants