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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for git submodules #469

Merged
merged 2 commits into from Jan 27, 2019

Conversation

4 participants
@ericcornelissen
Copy link
Contributor

ericcornelissen commented Oct 14, 2018

Implements build-in support for linting the commits of git sub modules, which currently doesn't work because sub modules don't have their own .git folder.

Description

I updated the read module to determine if it is running from a git sub module or not and find the COMMIT_EDITMSG accordingly. It still uses edit argument if present, otherwise it looks at .git and checks whether it is a directory (if it is, look for the COMMIT_EDITMSG as before) or a file, in which case it reads the file to figure out where the COMMIT_EDITMSG can be found.

Comments on my implementation are welcome, I'm happy to change it if needed 馃檪

Motivation and Context

This change is required in order to support linting the commits of git sub modules.

Fixes #448

Usage examples

n/a

How Has This Been Tested?

Tests are missing from this PR (at least for now) as it requires an update to the @commitlint/test package (support for sub modules in the git helper has to be added). However, I did create a test locally with a rough outline of what the update to the @commitlint/test might look like. You can find the required changes in this gist, simply checkout on this branch and replace the local files with the two in the gist and you can test the new feature.

The reason I opened this PR first is because I wanted to be sure this would be included before updating the @commitlint/test code and adding something that wasn't really needed...

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 change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes. (see "How Has This Been Tested?" for details)
  • All new and existing tests passed.
feat: add support for git submodules
No test included in this commit as this would require an update to
@commitlint/test to support submodules.
Show resolved Hide resolved @commitlint/read/src/index.js Outdated
fix: resolve path to commit message for git submodules
because `path.resolve` should always be used for relative paths.
@marionebl

This comment has been minimized.

Copy link
Member

marionebl commented Nov 30, 2018

Thanks for the PR @ericcornelissen! Could you provide us with a minimal test case you used to implement this? I'd like to include it as a fixture/test pair for the cli package.

@ericcornelissen

This comment has been minimized.

Copy link
Contributor Author

ericcornelissen commented Nov 30, 2018

I'm unsure what you mean @marionebl, in the PR description I outlined how it has been tested. If you need more detail, the test scenario I added is "get edit commit message from git submodule" which requires the function submodule in @commitlint/test.

What the test scenario does in plain English is:

  1. Bootstrap a git repository
  2. Add a submodule to that repository
  3. Make a commit in the submodule
  4. read from the submodule
  5. Verify read returned the commit message of the submodule's commit

Does that help?

@marionebl marionebl merged commit 83b1a47 into conventional-changelog:master Jan 27, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ericcornelissen ericcornelissen deleted the ericcornelissen:feat/448-support-submodules branch Jan 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment