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: drop commitlint usage #1901

Merged
merged 2 commits into from Dec 1, 2020
Merged

chore: drop commitlint usage #1901

merged 2 commits into from Dec 1, 2020

Conversation

trentm
Copy link
Member

@trentm trentm commented Nov 30, 2020

This removes usage of commitlint (via 'npm run lint:commit') to strictly
lint commit messages. The spirit was considered good, but too often was
a tripping hazard. The commit guidelines have been edited to attempt to
encourage focus the substance of good commit messages.

Fixes: #1887

This removes usage of commitlint (via 'npm run lint:commit') to strictly
lint commit messages. The spirit was considered good, but too often was
a tripping hazard. The commit guidelines have been edited to attempt to
encourage focus the substance of good commit messages.

Fixes: #1887
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Nov 30, 2020
@trentm trentm requested a review from astorm November 30, 2020 22:31
@trentm trentm added this to Planned in APM-Agents (OLD) via automation Nov 30, 2020
@apmmachine
Copy link
Collaborator

apmmachine commented Nov 30, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #1901 updated

  • Start Time: 2020-12-01T20:08:14.614+0000

  • Duration: 21 min 31 sec

Test stats 🧪

Test Results
Failed 0
Passed 16398
Skipped 0
Total 16398

@trentm trentm self-assigned this Nov 30, 2020
astorm
astorm previously approved these changes Dec 1, 2020
Copy link
Contributor

@astorm astorm left a comment

Choose a reason for hiding this comment

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

The PR itself looks solid, so approving.

Also, talking out loud, I noticed that that the CI check apm-ci/linting/pr-merge is still running/failing, so we'll want to figure what we need to do to get that off the list of checks made prior to a PR.

Copy link
Contributor

@astorm astorm left a comment

Choose a reason for hiding this comment

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

@trentm Question -- I just realized some of our jenkins tests invoke the commit lint -- specifically the test_basic.sh script. We'll probably want to remove this invocation as well https://github.com/elastic/apm-agent-nodejs/blob/master/.ci/scripts/test_basic.sh#L11

@astorm astorm dismissed their stale review December 1, 2020 18:50

Realized there was an invocation we'll need to update.

@trentm trentm merged commit e32d328 into master Dec 1, 2020
APM-Agents (OLD) automation moved this from Planned to Done Dec 1, 2020
@trentm trentm deleted the trentm/drop-commitlint branch December 1, 2020 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
Development

Successfully merging this pull request may close these issues.

drop commitlint
3 participants