Skip to content
This repository has been archived by the owner on Aug 13, 2019. It is now read-only.

STIJ-195: Added pre-commit hook support. #170

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

Conversation

meirege
Copy link
Contributor

@meirege meirege commented Nov 30, 2017

Description

Added a pre-commit hook integration to automatically test things like JS and SASS validation.

Motivation and Context

This might reduce errors in the production code.

How Has This Been Tested?

Did a local commit test.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have updated the CHANGELOG accordingly.
  • I have read the CONTRIBUTING document.

@meirege meirege added this to the 2.7.1 milestone Nov 30, 2017
@meirege meirege self-assigned this Nov 30, 2017
Copy link
Contributor

@daften daften left a comment

Choose a reason for hiding this comment

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

I'll add Peter to review this as well

postinstall.sh Outdated
cp -R ./node_modules/jquery ./public/styleguide/vendor/jquery;
cp -R ./node_modules/chosen-js ./public/styleguide/vendor/chosen-js;

cp ./scripts/pre-commit ./.git/hooks/pre-commit
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer something more clear for the git-hooks e.g. a .git-hooks folder which is also hidden on the filesystem by default.

Copy link

Choose a reason for hiding this comment

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

Use symlink instead of copy.
No support for windows users?

git stash -q --keep-index

# Test prospective commit
gulp validate;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer something that uses the package manager as wrapper to have uniformity. E.g. npm pre-commit. For PHP projects we could use composer pre-commit.

@daften daften requested a review from zero2one December 1, 2017 08:51
@meirege meirege changed the title STIJ-195: Added pre-commit hook support. ON HOLD: STIJ-195: Added pre-commit hook support. Dec 1, 2017
@meirege meirege modified the milestones: 2.7.1, 2.7.2 Dec 7, 2017
@meirege meirege changed the title ON HOLD: STIJ-195: Added pre-commit hook support. STIJ-195: Added pre-commit hook support. Dec 7, 2017
@meirege meirege modified the milestones: 2.7.4, 2.7.5 Dec 7, 2017
@Jeroen005 Jeroen005 modified the milestones: 2.7.5, 2.7.6 Dec 13, 2017
@meirege meirege modified the milestones: 2.7.6, 2.7.7 Dec 14, 2017
@meirege meirege modified the milestones: 2.7.7, 2.7.8, 2.8.0, 2.9.0 Jan 18, 2018
@meirege meirege modified the milestones: 2.9.0, 3.0.0 Jan 30, 2018
Copy link

@zero2one zero2one left a comment

Choose a reason for hiding this comment

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

No longer relevant?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants