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.

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

chore: add commitlint and husky #128

Merged

Conversation

OlivierAlbertini
Copy link
Member

Resolves #117

Signed-off-by: Olivier Albertini olivier.albertini@montreal.ca

@codecov-io
Copy link

codecov-io commented Jul 24, 2019

Codecov Report

Merging #128 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #128   +/-   ##
=======================================
  Coverage   98.74%   98.74%           
=======================================
  Files          29       29           
  Lines        1915     1915           
  Branches      221      221           
=======================================
  Hits         1891     1891           
  Misses         24       24

@OlivierAlbertini
Copy link
Member Author

We still need to have someone who add gitcop (or something equivalent)
Here the format that need to be set in gitcop if you want to make the scope optional %{type}(?%?{scope})?: %{description}

Copy link
Contributor

@draffensperger draffensperger left a comment

Choose a reason for hiding this comment

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

Will this be triggered by the CircleCI build?

@OlivierAlbertini
Copy link
Member Author

OlivierAlbertini commented Jul 24, 2019

Will this be triggered by the CircleCI build?

No. This will be triggered has a commit-msg hook (when you do git commit) to ensure that is standard. But it's not 100% safe, it can be skipped as well, e.g editing directly through Github... gitcop would be useful for that.

commitlint + husky is good to prevent that and avoid roundtrip

@mayurkale22
Copy link
Member

If I don't add commit message in the correct format, at what stage I should expect to see the failure message? Is it when I open the pull request?

@OlivierAlbertini
Copy link
Member Author

you won't be able to write a commit message if it's not standard , e.g

I do git commit -am "wrong message", you will see an error and the message won't be comitted.

@mayurkale22
Copy link
Member

you won't be able to write a commit message if it's not standard , e.g

I do git commit -am "wrong message", you will see an error and the message won't be comitted.

hmm I see. Do you think we should add this in CONTRIBUTING.md guide?

@OlivierAlbertini
Copy link
Member Author

sure @mayurkale22, this is a good idea.

@rochdev
Copy link
Member

rochdev commented Jul 24, 2019

I'm not a fan of doing this on commit. When committing WIP I generally will defer thinking about the commit message to a later rebase. Any way we can trigger this at the PR stage as mentioned above?

@OlivierAlbertini
Copy link
Member Author

OlivierAlbertini commented Jul 24, 2019

You can do git commit -am "WIP" --no-verify to skip this When committing WIP

@OlivierAlbertini
Copy link
Member Author

I added Conventional commit section to CONTRIBUTING.md under Before you start, let me know if anything is unclear.

Change CONTRIBUTING.md
Resolves open-telemetry#117

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>
@mayurkale22
Copy link
Member

@draffensperger @rochdev @danielkhan Please approve the PR, if everything looks good.

Copy link
Contributor

@draffensperger draffensperger left a comment

Choose a reason for hiding this comment

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

Thanks @OlivierAlbertini for setting this up!

Copy link
Member

@rochdev rochdev left a comment

Choose a reason for hiding this comment

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

LGTM. Does this apply to rebase as well?

@OlivierAlbertini
Copy link
Member Author

OlivierAlbertini commented Jul 25, 2019

yes

➜ git rebase -i af8a7e3450c8b1fb94c8b2182a90b109e77c89a1
husky > commit-msg (node v10.9.0)
⧗   input: ch: add commitlint and husky

Change CONTRIBUTING.md

Resolves #117

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>
✖   type must be one of [ci, feat, fix, docs, style, refactor, perf, test, revert, chore] [type-enum]
⚠   footer must have leading blank line [footer-leading-blank]

@mayurkale22 mayurkale22 merged commit 44214ac into open-telemetry:master Jul 25, 2019
@OlivierAlbertini OlivierAlbertini deleted the feature/commitlint branch August 30, 2019 22:29
dyladan added a commit to dyladan/opentelemetry-js that referenced this pull request Sep 9, 2022
Change CONTRIBUTING.md
Resolves open-telemetry#117

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
…oader-8.x

chore(deps): update dependency ts-loader to v8
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.

Integrate commitlint and husky
5 participants