Skip to content
This repository has been archived by the owner on May 4, 2022. It is now read-only.

style: wrap recent changelog at 80 chars per line #629

Merged
merged 1 commit into from
Nov 30, 2017

Conversation

apetro
Copy link
Contributor

@apetro apetro commented Nov 29, 2017

Wrap recent changelog lines at 80 characters per line.


Contributor License Agreement adherence:

Copy link
Contributor

@vertein vertein left a comment

Choose a reason for hiding this comment

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

Triggering some linting errors. +1 when CI checks successful.

@apetro
Copy link
Contributor Author

apetro commented Nov 29, 2017

If only there were some way this linting could automatically run for me when I locally commit... :)

@apetro
Copy link
Contributor Author

apetro commented Nov 29, 2017

(That next failure was because my commit message on the linting fix ended with a period and so itself failed commit message linting. If only there were some way my commit messages could be linted locally when I commit them... :) ).

So, the helpful version of this joke is this: that there are tradeoffs between local and CI linting, and I'm navigating them, badly. One root problem here is that this linting isn't working for me locally -- I imagine npm run lint-md is supposed to let me lint my Markdown locally, but locally my latest error message when I try to run that is Cannot find module 'trough'.

@vertein
Copy link
Contributor

vertein commented Nov 29, 2017

Ahh, the old git commit messages aren't sentences gotcha.

Have you tried npm install before running npm run lint-md? I'll pull down the code and see if it works for me.
Edit - Works for me, and now I see it works for you too.

@apetro
Copy link
Contributor Author

apetro commented Nov 29, 2017

So, yes, I had tried npm install, twice. Tried it one more time now. And of course now npm run lint-md works just fine for me locally.

@apetro
Copy link
Contributor Author

apetro commented Nov 29, 2017

(And apparently the next failure was because my commit message led with 4 after the scope. This will eventually be funny. Squashed so I only have to get 1 commit message right.)

@ChristianMurphy
Copy link
Contributor

ChristianMurphy commented Nov 29, 2017

Hey @apetro 👋
Chiming in a bit late here.
You may enjoy using commitizen npm run cz (availible now) or commitlint prompt npm run commit (coming as part of #621).
Both provides a commandline WYSIWYG for crafting conventional commit messages.

Also, it may be valuable if you share your experiences and thoughts on how to improve the commit message experience at https://github.com/marionebl/commitlint/issues/94.

@ChristianMurphy
Copy link
Contributor

So, yes, I had tried npm install, twice. Tried it one more time now. And of course now npm run lint-md works just fine for me locally.

I'd check which version of npm is being run versions 5.1.0+ are much more consistent with installing modules than previous versions.
ref: https://docs.npmjs.com/troubleshooting/try-the-latest-stable-version-of-npm

Its also worth noting that running npm install multiple times can lead to some non-determinism in the structure/versions in the node_modules folder.
For the most consistent results rm -rf node_modules && npm install.
ref: https://docs.npmjs.com/how-npm-works/npm3-nondet

@ChristianMurphy
Copy link
Contributor

If only there were some way my commit messages could be linted locally when I commit them

That's the dream. 🙂
Its worth noting that the webhooks removed as part of the /issues/535 purge of githooks, can still be manually added and maintained within a local setup of uPortal-app-framework.
It just take more effort and initiative.

@apetro apetro merged commit 32e23b3 into uPortal-Attic:master Nov 30, 2017
@apetro apetro deleted the wrap-changelong-at-80-char branch November 30, 2017 14:55
@vertein vertein added this to the 7.0.0 milestone Nov 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants