Git pre-push checks for UI #3072
Conversation
|
|
||
| # Check that the branch starts with `ui` or `js` | ||
| # and that `npm` is installed | ||
| if [[ "$branch" =~ ^(ui|js) ]] && npm --version 1>/dev/null 2>/dev/null; |
There was a problem hiding this comment.
I'd rather do it on everything, it is fast enough - plus, we are doing more-and-more of the combined PRs especially in the jsapi realm. I like the copy approach, just think it should cover all branches when a developer switches this on.
There was a problem hiding this comment.
Okay, agreed. So what about this :
- On
postinstall, copy the git hook pre-push script to the right folder (like that everyone using npm will have it) - The script itself only checks if
npmis installed, as well aseslint(globally or in thenode_modules/bindirectory)
jacogr
added
A5-grumble 🔥
ngotchac
added
A0-pleasereview 🤓
jacogr
added
A8-looksgood 🦄
| "test": "mocha 'src/**/*.spec.js'", | ||
| "test:coverage": "istanbul cover _mocha -- 'src/**/*.spec.js'", | ||
| "test:e2e": "mocha 'src/**/*.e2e.js'" | ||
| "test:e2e": "mocha 'src/**/*.e2e.js'", | ||
| "postinstall": "cp scripts/pre-push ../.git/hooks/" |
There was a problem hiding this comment.
Not sure it's worth pulling in another dev dependency, but there's husky.
| @@ -0,0 +1,15 @@ | |||
| #!/bin/bash | |||
|
|
|||
| base_dir=$(git rev-parse --show-toplevel) | |||
There was a problem hiding this comment.
Why not use npm prefix here?
There was a problem hiding this comment.
I guess this would be invalidated anyway with husky
jacogr
added
A1-onice 🌨
|
Looks good. Marked it onice since fist want a proper test (after release) when GitLab is up and running again. Don't want fireworks. So as it stands, looks good, just want to make sure we run all the checks and is prepared for it. (Specifically since we commit from CI) In short - once GitLab is humming again, can merge, test & firefight if need-be. |
|
Seems fair. Note that with |
|
Here's a UI screenshot |
|
And another |
|
and another |
|
and another |
|
|
gavofyork
added
M1-ci 🙉
|
gitlab seems to be working again |
|
Thanks will get this one in the morning. (Want to make sure we are there to watch - just in-case) |
This would address partially #2291
A new script in
js/scripts/pre-pushthat should be copied to.git/hooks.It runs linting for the js files if the current branch name starts with
jsorui, and ifnpmis installed.This would prevent this hook to be executed by everybody, but only devs working on the UI.
NB : the linter runs in about 3sec