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.

Sign up for GitHub

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

Jump to bottom

Disabling BLT commit message #2582

Closed
justinlevi opened this issue Feb 27, 2018 · 14 comments
Closed

Disabling BLT commit message #2582

justinlevi opened this issue Feb 27, 2018 · 14 comments
Labels
Support A support request

Comments

@justinlevi
Copy link
Contributor

justinlevi commented Feb 27, 2018 •
edited
Loading

Bringing back an old issue...
#2128

My system information:

  • Operating system type: Mac OSX 10.13.3
  • BLT version: 9.0-beta2 ----> 9.0-beta3

Output of blt doctor:

+---------------------------+--------------------------------------------------------------+
| Property                  | Value                                                        |
+---------------------------+--------------------------------------------------------------+
| %paths.%root              | /Users/justinwinter/Sites/d8d/docroot                        |
| %paths.%site              | sites/default                                                |
| %paths.%modules           | sites/all/modules                                            |
| %paths.%themes            | sites/all/themes                                             |
| %paths.%config-sync       | /Users/justinwinter/Sites/d8d/config/default                 |
| %paths.%files             | sites/default/files                                          |
| %paths.%temp              | /tmp                                                         |
| %paths.%private           | /Users/justinwinter/Sites/d8d/files-private                  |
| admin-theme               | seven                                                        |
| alias-searchpaths.0       | /Users/justinwinter/Sites/d8d/drush/sites                    |
| blt-version               | 9.0.0-beta3                                                  |
| bootstrap                 | Successful                                                   |
| composer-version          | Composer version 1.5.2 2017-09-11                            |
|                           | 16:59:25                                                     |
| config-sync               | /Users/justinwinter/Sites/d8d/config/default                 |
| db-driver                 | mysql                                                        |
| db-hostname               | localhost                                                    |
| db-name                   | d8d                                                          |
| db-password               | drupal                                                       |
| db-port                   | 3306                                                         |
| db-status                 | Connected                                                    |
| db-username               | drupal                                                       |
| drupal-settings-file      | sites/default/settings.php                                   |
| drupal-version            | 8.5.0-rc1                                                    |
| drush-alias-files.0       | /Users/justinwinter/Sites/d8d/drush/sites/d8d.site.yml       |
| drush-conf.0              | /Users/justinwinter/Sites/d8d/vendor/drush/drush/drush.yml   |
| drush-conf.1              | /Users/justinwinter/Sites/d8d/drush/drush.yml                |
| drush-conf.2              | /Users/justinwinter/Sites/d8d/docroot/sites/default/local.dr |
|                           | ush.yml                                                      |
| drush-script              | /Users/justinwinter/Sites/d8d/vendor/bin/drush               |
| drush-temp                | /tmp                                                         |
| drush-version             | 9.2.0                                                        |
| files                     | sites/default/files                                          |
| install-profile           | d8d                                                          |
| modules                   | sites/all/modules                                            |
| php-bin                   | /usr/local/Cellar/php71/7.1.10_21/bin/php                    |
| php-conf.0                | /usr/local/etc/php/7.1/php.ini                               |
| php-os                    | Darwin                                                       |
| private                   | /Users/justinwinter/Sites/d8d/files-private                  |
| root                      | /Users/justinwinter/Sites/d8d/docroot                        |
| site                      | sites/default                                                |
| stacks.drupal-vm.inited   | false                                                        |
| stacks.dev-desktop.inited | false                                                        |
| temp                      | /tmp                                                         |
| theme                     | bartik                                                       |
| themes                    | sites/all/themes                                             |
| uri                       | http://d8d.loc                                               |
+---------------------------+--------------------------------------------------------------+
+--------------------------------------+--------------------------------------------------------------+
| Check                                | Problem                                                      |
+--------------------------------------+--------------------------------------------------------------+
| ConfigCheck:checkGitConfig           | Git repositories are not defined in blt.yml.                 |
|                                      |   Add values for git.remotes to blt.yml to enabled automated |
|                                      | deployment.                                                  |
| NodeCheck:checkNodeVersionFileExists | Neither .nvmrc nor .node-version file found in repo root.    |
+--------------------------------------+--------------------------------------------------------------+

When I run this command:

$ `git commit -m "blt upgrade and travis config"`

I get the following output:

Executing .git/hooks/pre-commit...
Executing .git/hooks/commit-msg...
Validating commit message syntax...
PHP Warning:  preg_match(): Empty regular expression in /Users/justinwinter/Sites/d8d/vendor/acquia/blt/src/Robo/Commands/Git/GitCommand.php on line 25

Warning: preg_match(): Empty regular expression in /Users/justinwinter/Sites/d8d/vendor/acquia/blt/src/Robo/Commands/Git/GitCommand.php on line 25
[error]  Invalid commit message! 
Commit messages must conform to the regex 
[notice] To disable this command, see http://blt.rtfd.io/en/8.x/readme/extending-blt/#disabling-a-command
[notice] To customize git hooks, see http://blt.rtfd.io/en/8.x/readme/extending-blt/#setupgit-hooks.
soundslides:d8d justinwinter$ git commit -m "blt upgrade and travis config"

And I expected this to happen:

I expect the pre-commit requirement to be ignored.

Here is my blt.yml, including the disable-targets and git hooks > pre-commit configuration settings that worked prior to the update.

project:
  machine_name: d8d
  prefix: BLT
  human_name: 'BLTed 8'
  profile:
    name: lightning
  local:
    protocol: http
    hostname: 'local.${project.machine_name}.com'
git:
  default_branch: master
  remotes: {  }
  pre-commit: false
  commit-msg: false
  hooks:
    pre-commit: false
drush:
  aliases:
    remote: '${project.machine_name}.test'
    local: self
    ci: self
  default_alias: '${drush.aliases.local}'
modules:
  local:
    enable: [dblog, devel, seckit, views_ui]
    uninstall: [acquia_connector, shield]
  ci:
    enable: {  }
    uninstall: [acquia_connector, shield]
  dev:
    enable: [acquia_connector, shield]
    uninstall: {  }
  test:
    enable: [acquia_connector, shield]
    uninstall: [devel, views_ui]
  prod:
    enable: [acquia_connector, shield]
    uninstall: [devel, views_ui]
disable-targets:
  validate:
    phpcs: true
  git:
    commit-msg: true
    pre-commit: true
@grasmash
Copy link
Contributor

Does vendor/acquia/blt/config/build.yml contain a value for git.commit-msg.pattern?

https://github.com/acquia/blt/blob/9.x/config/build.yml#L96

@grasmash grasmash added the Support A support request label Feb 27, 2018
@justinlevi
Copy link
Contributor Author

justinlevi commented Feb 27, 2018 •
edited
Loading

I realize I might be in the minority here, but I'm wondering if it might make more sense to disable the commit messages by default and make it optional to turn them on. Feels a bit heavy handed to me to require a commit message. From my experience, this is a self policing issue in that really bad commit messages are pretty useless and should be encouraged against as a culture. I also feel like it might be added friction for new user adoption as it takes a bit of digging to find out how to disable this setting.

@justinlevi
Copy link
Contributor Author

@grasmash Yes, it does.

git:
  # The value of a hook should be the file path to a directory containing an
  # executable file named after the hook.
  # Changing a hook value to 'false' will disable it.
  hooks:
    pre-commit: ${blt.root}/scripts/git-hooks
    commit-msg: ${blt.root}/scripts/git-hooks
  commit-msg:
    # Commit messages must conform to this pattern.
    pattern: "/(^${project.prefix}-[0-9]+(: )[^ ].{15,}\\.)|(Merge branch (.)+)/"

grasmash added a commit that referenced this issue Feb 27, 2018
@grasmash
Connects to #2582: Disabling BLT commit message.
8459b6f
@grasmash
Copy link
Contributor

Well, that warning is puzzling.

Can you share the output of blt config:dump ?

@justinlevi
Copy link
Contributor Author

super bizarre... when I first ran $ blt config:dump, nothing was returned. I removed my vendor folder and ran composer update again and here's what I have:

soundslides:d8d justinwinter$ blt config:dump
+-----------------------------------------+--------------------------------------------------------------+
| Property                                | Value                                                        |
+-----------------------------------------+--------------------------------------------------------------+
| behat.config                            | /Users/justinwinter/Sites/d8d/tests/behat/local.yml          |
| behat.profile                           | local                                                        |
| behat.selenium.port                     | 4444                                                         |
| behat.selenium.url                      | http://127.0.0.1:4444/wd/hub                                 |
| behat.chrome.port                       | 9222                                                         |
| behat.chrome.args                       |                                                              |
| behat.paths.0                           | /Users/justinwinter/Sites/d8d/tests/behat                    |
| behat.tags                              | ~ajax&&~experimental&&~lightningextension                    |
| behat.extra                             |                                                              |
| behat.web-driver                        | chrome                                                       |
| bin.path                                | vendor/bin                                                   |
| blt.root                                | /Users/justinwinter/Sites/d8d/vendor/acquia/blt              |
| blt.update.ignore-existing-file         | /Users/justinwinter/Sites/d8d/vendor/acquia/blt/scripts/blt/ |
|                                         | ignore-existing.txt                                          |
| blt.config-files.project                | /Users/justinwinter/Sites/d8d/blt/blt.yml                    |
| blt.config-files.local                  | /Users/justinwinter/Sites/d8d/blt/local.blt.yml              |
| blt.config-files.example-local          | /Users/justinwinter/Sites/d8d/blt/example.local.blt.yml      |
| blt.config-files.schema-version         | /Users/justinwinter/Sites/d8d/blt/.schema_version            |
| blt.command-cache-dir                   | /Users/justinwinter/Sites/d8d/vendor/acquia/blt/cache/comman |
|                                         | ds                                                           |
| cm.strategy                             | config-split                                                 |
| cm.core.path                            | ../config                                                    |
| cm.core.key                             | sync                                                         |
| cm.core.dirs.sync.path                  | ../config/default                                            |
| cm.features.no-overrides                | true                                                         |
| command-hooks.frontend-reqs.dir         | /Users/justinwinter/Sites/d8d/docroot                        |
| command-hooks.frontend-reqs.command     |                                                              |
| command-hooks.frontend-assets.dir       | /Users/justinwinter/Sites/d8d/docroot                        |
| command-hooks.frontend-assets.command   |                                                              |
| command-hooks.frontend-test.dir         | /Users/justinwinter/Sites/d8d/docroot                        |
| command-hooks.frontend-test.command     |                                                              |
| command-hooks.pre-commit.dir            | /Users/justinwinter/Sites/d8d/docroot                        |
| command-hooks.pre-commit.command        |                                                              |
| command-hooks.pre-config-import.dir     | /Users/justinwinter/Sites/d8d/docroot                        |
| command-hooks.pre-config-import.command |                                                              |
| command-hooks.post-deploy-build.dir     | /Users/justinwinter/Sites/d8d/deploy/docroot                 |
| command-hooks.post-deploy-build.command |                                                              |
| command-hooks.post-setup-build.dir      | /Users/justinwinter/Sites/d8d/docroot                        |
| command-hooks.post-setup-build.command  |                                                              |
| command-hooks.post-deploy.dir           | /Users/justinwinter/Sites/d8d/docroot                        |
| command-hooks.post-deploy.command       |                                                              |
| composer.bin                            | /Users/justinwinter/Sites/d8d/vendor/bin                     |
| composer.extra                          |                                                              |
| deploy.build-dependencies               | true                                                         |
| deploy.dir                              | /Users/justinwinter/Sites/d8d/deploy                         |
| deploy.exclude_file                     | /Users/justinwinter/Sites/d8d/vendor/acquia/blt/scripts/blt/ |
|                                         | deploy/deploy-exclude.txt                                    |
| deploy.exclude_additions_file           | /Users/justinwinter/Sites/d8d/blt/deploy-exclude-additions.t |
|                                         | xt                                                           |
| deploy.gitignore_file                   | /Users/justinwinter/Sites/d8d/vendor/acquia/blt/scripts/blt/ |
|                                         | deploy/.gitignore                                            |
| deploy.git.failOnDirty                  | true                                                         |
| disable-targets.validate.phpcs          | true                                                         |
| disable-targets.git.commit-msg          | true                                                         |
| disable-targets.git.pre-commit          | true                                                         |
| docroot                                 | /Users/justinwinter/Sites/d8d/docroot                        |
| drupal.account.mail                     | no-reply@acquia.com                                          |
| drupal.locale                           | en                                                           |
| drupal.local_settings_file              | /Users/justinwinter/Sites/d8d/docroot/sites/default/settings |
|                                         | /local.settings.php                                          |
| drupal.settings_file                    | /Users/justinwinter/Sites/d8d/docroot/sites/default/settings |
|                                         | .php                                                         |
| drupal.db.database                      | drupal                                                       |
| drupal.db.username                      | drupal                                                       |
| drupal.db.password                      | drupal                                                       |
| drupal.db.host                          | localhost                                                    |
| drupal.db.port                          | 3306                                                         |
| drush.bin                               | /Users/justinwinter/Sites/d8d/vendor/bin/drush               |
| drush.dir                               | /Users/justinwinter/Sites/d8d/docroot                        |
| drush.alias-dir                         | /Users/justinwinter/Sites/d8d/drush/sites                    |
| drush.sanitize                          | true                                                         |
| drush.aliases.remote                    | d8d.test                                                     |
| drush.aliases.local                     | d8d.loc                                                      |
| drush.aliases.ci                        | self                                                         |
| drush.default_alias                     | d8d.loc                                                      |
| drush.alias                             | d8d.loc                                                      |
| environment                             | local                                                        |
| git.hooks.pre-commit                    | false                                                        |
| git.hooks.commit-msg                    | false                                                        |
| git.commit-msg                          | false                                                        |
| git.default_branch                      | master                                                       |
| git.pre-commit                          | false                                                        |
| modules.local.enable.0                  | dblog                                                        |
| modules.local.enable.1                  | devel                                                        |
| modules.local.enable.2                  | seckit                                                       |
| modules.local.enable.3                  | views_ui                                                     |
| modules.local.uninstall.0               | acquia_connector                                             |
| modules.local.uninstall.1               | shield                                                       |
| modules.ci.uninstall.0                  | acquia_connector                                             |
| modules.ci.uninstall.1                  | shield                                                       |
| modules.dev.enable.0                    | acquia_connector                                             |
| modules.dev.enable.1                    | shield                                                       |
| modules.test.enable.0                   | acquia_connector                                             |
| modules.test.enable.1                   | shield                                                       |
| modules.test.uninstall.0                | devel                                                        |
| modules.test.uninstall.1                | views_ui                                                     |
| modules.prod.enable.0                   | acquia_connector                                             |
| modules.prod.enable.1                   | shield                                                       |
| modules.prod.uninstall.0                | devel                                                        |
| modules.prod.uninstall.1                | views_ui                                                     |
| multisites.0                            | default                                                      |
| options.decorated                       | true                                                         |
| options.interactive                     | true                                                         |
| options.progress-delay                  | 2                                                            |
| options.simulate                        | false                                                        |
| options.help                            | false                                                        |
| options.quiet                           | false                                                        |
| options.verbose                         | false                                                        |
| options.version                         | false                                                        |
| options.ansi                            | false                                                        |
| options.no-ansi                         | false                                                        |
| options.no-interaction                  | false                                                        |
| options.yes                             | false                                                        |
| phpcbf.filesets.0                       | files.php.custom.modules                                     |
| phpcbf.filesets.1                       | files.php.tests                                              |
| phpcbf.filesets.2                       | files.php.custom.themes                                      |
| phpcbf.filesets.3                       | files.frontend.custom.themes                                 |
| phpcs.standard                          | Drupal,DrupalPractice                                        |
| phpunit.0.path                          | /Users/justinwinter/Sites/d8d/tests/phpunit                  |
| phpunit.0.class                         | ExampleTest                                                  |
| phpunit.0.file                          | ExampleTest.php                                              |
| phpunit.0.config                        |                                                              |
| project.local.uri                       | http://local.d8d.com                                         |
| project.local.protocol                  | http                                                         |
| project.local.hostname                  | local.d8d.com                                                |
| project.machine_name                    | d8d                                                          |
| project.prefix                          | BLT                                                          |
| project.human_name                      | BLTed 8                                                      |
| project.profile.name                    | lightning                                                    |
| repo.root                               | /Users/justinwinter/Sites/d8d                                |
| reports.localDir                        | /Users/justinwinter/Sites/d8d/reports                        |
| reports.remoteDir                       | reports                                                      |
| setup.strategy                          | install                                                      |
| setup.dump-file                         |                                                              |
| site                                    | default                                                      |
| sync.files                              | false                                                        |
| sync.exclude-paths.0                    | styles                                                       |
| sync.exclude-paths.1                    | css                                                          |
| sync.exclude-paths.2                    | js                                                           |
| sync.commands.0                         | source:build:composer                                        |
| sync.commands.1                         | blt:init:settings                                            |
| sync.commands.2                         | drupal:sync:db                                               |
| sync.commands.3                         | drupal:update                                                |
| sync.commands.4                         | source:build:frontend                                        |
| tests.run-server                        | false                                                        |
| tests.server.port                       | 8888                                                         |
| tests.server.url                        | http://127.0.0.1:8888                                        |
| validate.deprecated.filesets.0          | files.php.custom.modules                                     |
| validate.lint.filesets.0                | files.php.custom.modules                                     |
| validate.lint.filesets.1                | files.php.custom.themes                                      |
| validate.lint.filesets.2                | files.php.tests                                              |
| validate.twig.filesets.0                | files.twig                                                   |
| validate.yaml.filesets.0                | files.yaml                                                   |
| vm.enable                               | false                                                        |
| vm.config                               | /Users/justinwinter/Sites/d8d/box/config.yml                 |
| vm.vagrant.hostname                     | local.d8d.com                                                |
+-----------------------------------------+--------------------------------------------------------------+

@grasmash
Copy link
Contributor

grasmash commented Feb 27, 2018 •
edited
Loading

Interesting. git.commit-msg.pattern is not set. Does vendor/acquia/blt/config/build.yml still contain a value for git.commit-msg.pattern after re-creating vendor?

@justinlevi
Copy link
Contributor Author

For the time being, because I gotta move on with some other stuff, I'm just going to throw this in composer.json

    "scripts": {
        "post-install-cmd": [
            "rm -f .git/hooks/commit-msg",
            "rm -f .git/hooks/pre-commit"
        ],
        "post-update-cmd": [
            "rm -f .git/hooks/commit-msg",
            "rm -f .git/hooks/pre-commit"
        ]
    },

grasmash added a commit that referenced this issue Feb 28, 2018
@grasmash
Connects to #2582: Disabling BLT commit message. (#2585)
9de18fc 
* Connects to #2582: Disabling BLT commit message.

* Update GitCommand.php
@grasmash
Copy link
Contributor

grasmash commented Mar 1, 2018

I'm not able to reproduce this issue, but I added some output to assist with debugging if we hit it later. For now, I'd suggest setting git.hooks to [] rather than adding composer scripts.

@grasmash grasmash closed this as completed Mar 1, 2018
@jrbeeman
Copy link
Contributor

I have also run into this issue in a new project on BLT 9.0.5. Per the documentation, I added the following to blt/blt.yml, but commit message validation is still occurring.

disable-targets:
  git:
    commit-msg: true

@jrbeeman jrbeeman reopened this Mar 20, 2018
@grasmash
Copy link
Contributor

Ah, this is probably because the command names have changed. The docs need to be updated.

@jrbeeman
Copy link
Contributor

Looking at vendor/acquia/blt/config/build.yml, I read the comments around the git.hooks config as implying that I should instead add the following to blt.yml

git:
  hooks:
    commit-msg: false

With the implication that I would not use disable-targets at all. Is that correct? If so, I'll submit a PR to change the docs.

@grasmash
Copy link
Contributor

grasmash commented Mar 21, 2018 •
edited
Loading

That is the best way to disable the git hook.

Though your initial attempt also should have worked with something like...

disable-targets:
	internal:
		git-hook:
			execute:
				pre-commit: true

Obviously that's not preferable.

jrbeeman added a commit to jrbeeman/blt that referenced this issue Mar 21, 2018
@jrbeeman
acquia#2582: Update documentation around git commit-msg hook.
9125a21
@jrbeeman jrbeeman mentioned this issue Mar 21, 2018
Closed
grasmash added a commit that referenced this issue Mar 22, 2018
@grasmash
Fixes #2582: Config documentation fixes.
b7864b9
@jnettik
Copy link

jnettik commented Apr 20, 2018

I'd like to echo the earlier suggestion for disabling this setting by default. I'm currently trying out BLT for the first time for an upcoming project and this tripped me up for a while. It's not clear that a local environment setup would police the commit messages to begin with, and it feels very unlikely that any project's convention would be to prefix issues with BLT. Reading through the documentation at http://blt.readthedocs.io/en/9.x/readme/extending-blt/#commit-msg, it wasn't clear how to disable this. The config from the last message did end up working, but in that same message it says "Obviously that's not preferable" implying there should be a better way to disable this.

I do think this feature is really handy to have available, to easily configure a standard commit message for teams. The way it's setup out of the box just makes it a very frustrating obstacle in getting setup.

@stuartabrown stuartabrown mentioned this issue Nov 15, 2018
Closed
@capysara
Copy link

There's a little more detail here:

Add this snippet right underneath git.remotes configurations in the blt/blt.yml file.

hooks:
     commit-msg: false
     pattern: false

And then initiate the config change:
blt blt:init:git-hooks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Support A support request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jrbeeman @grasmash @jnettik @justinlevi @capysara