Add markdownlint support #740

adamconnelly · 2019-10-12T11:51:14Z

Fixes #453

Proposed Changes

Added markdownlint via npm, and added a basic configuration file.
At the moment I've just setup the defaults, but disabled the line
length rule for code blocks and tables.
Updated the contributing guide to include information on running it.
Added a new stage to the build pipeline to run the linting.

@tomkerkhove I didn't want to take this too far, but I've got another couple of suggestions:

We can use Husky to create shared git hooks. What this would let us do is define a pre-commit hook that automatically runs markdownlint. This means that people get notified immediately if they've made a linting error, without having to wait for a build to run on GitHub.
We could potentially use Prettier to automatically format our markdown. What this would mean is that if you made a simple mistake that it can fix like a line being longer than 80 characters, we'd run Prettier as part of the Husky pre-commit hook, and automatically fix any simple problems before linting. This would mean less manual fixing of simple formatting issues.

promitor-bot · 2019-10-12T11:57:48Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

promitor-bot · 2019-10-12T12:05:18Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

promitor-bot · 2019-10-12T12:12:16Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

promitor-bot · 2019-10-12T12:16:44Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

promitor-bot · 2019-10-12T12:24:35Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

promitor-bot · 2019-10-12T12:26:36Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

promitor-bot · 2019-10-12T12:31:02Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

promitor-bot · 2019-10-12T12:35:36Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

- Added markdownlint via npm, and added a basic configuration file. At the moment I've just setup the defaults, but disabled the line length rule for code blocks and tables. - Updated the contributing guide to include information on running it. - Added a new stage to the build pipeline to run the linting. Fixes tomkerkhove#453

I've tried to only fix the issues highlighted by markdownlint in this commit, and keep the files as close as possible to how they were before these changes, although I did fix a couple of typos that I spotted as I was going through. I've had to increase the heading level by one on most of the files because they had multiple h1s.

promitor-bot · 2019-10-12T12:38:53Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

adamconnelly · 2019-10-12T12:39:35Z

Sorry about all the noise in this PR - it took me a while to get the command running properly in Azure Pipelines.

Here's an example failing build: https://tomkerkhove.visualstudio.com/Promitor/_build/results?buildId=1511&view=logs&j=a0162c90-9618-5d58-2cc8-c63b24381f83&t=50aea722-6967-5a54-fc98-844cbba5a109&l=15

promitor-bot · 2019-10-12T12:40:14Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

build/azure-devops/scraper-ci.yml

charts/promitor-agent-scraper/README.md

tomkerkhove · 2019-10-12T15:54:30Z

Thanks for the PR @adamconnelly! Is there a reason why the guideline is to only have x characters on one line?

We can use Husky to create shared git hooks. What this would let us do is define a pre-commit hook that automatically runs markdownlint. This means that people get notified immediately if they've made a linting error, without having to wait for a build to run on GitHub.

We could potentially use Prettier to automatically format our markdown. What this would mean is that if you made a simple mistake that it can fix like a line being longer than 80 characters, we'd run Prettier as part of the Husky pre-commit hook, and automatically fix any simple problems before linting. This would mean less manual fixing of simple formatting issues.

I'd suggest to create a dedicated issue for this discussion, is that ok for you?

But for now locally this will not be enforced until you create a PR where the linting is happening, is that correct? If so, do we still need the edit to .github/CONTRIBUTING.md?

tomkerkhove · 2019-10-12T15:56:29Z

Sorry about all the noise in this PR - it took me a while to get the command running properly in Azure Pipelines.

Here's an example failing build: tomkerkhove.visualstudio.com/Promitor/_build/results?buildId=1511&view=logs&j=a0162c90-9618-5d58-2cc8-c63b24381f83&t=50aea722-6967-5a54-fc98-844cbba5a109&l=15

No worries, thanks!

adamconnelly · 2019-10-12T16:28:44Z

I think the 80 character limit is just to make the files readable when you're viewing them in a text editor, rather than in a browser. If you view the docs in GitHub or on the site, it word wraps and enforces a gutter on either side to make them readable, but if you view them in a text editor it won't necessarily do that, so you can end up with really long lines.

But it's just a default setting, so we can change it if you want.

adamconnelly · 2019-10-12T16:36:26Z

@tomkerkhove I've just pushed another commit that reverts the change to CONTRIBUTING.md, renamed the pipeline steps and adds information on running markdownlint to the docs README.

You're correct - it'll only run in CI, or if you manually run npm run markdownlint for now.

promitor-bot · 2019-10-12T16:37:21Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr740

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr740 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr740

You can find a CI version of our Helm chart on hub.helm.sh

README.md

tomkerkhove · 2019-10-14T11:50:09Z

Thanks Adam!

adamconnelly requested a review from tomkerkhove as a code owner October 12, 2019 11:51

adamconnelly force-pushed the markdownlint branch from 503d186 to 1080a4c Compare October 12, 2019 11:55

adamconnelly force-pushed the markdownlint branch from d1e13ca to 635ae5a Compare October 12, 2019 12:33

adamconnelly added 2 commits October 12, 2019 13:36

adamconnelly force-pushed the markdownlint branch 2 times, most recently from d696063 to e7aabb7 Compare October 12, 2019 12:38

tomkerkhove added automation documentation All issues related to documentation on how to use it, deploy it, etc labels Oct 12, 2019

tomkerkhove reviewed Oct 12, 2019

View reviewed changes

build/azure-devops/scraper-ci.yml Show resolved Hide resolved

build/azure-devops/scraper-ci.yml Outdated Show resolved Hide resolved

build/azure-devops/scraper-ci.yml Outdated Show resolved Hide resolved

charts/promitor-agent-scraper/README.md Show resolved Hide resolved

fixup! Add markdownlint to project

6dd1185

adamconnelly mentioned this pull request Oct 13, 2019

Use Husky to share git hooks #741

Closed

1 task

tomkerkhove approved these changes Oct 14, 2019

View reviewed changes

README.md Show resolved Hide resolved

tomkerkhove merged commit 3ba6655 into tomkerkhove:master Oct 14, 2019

adamconnelly deleted the markdownlint branch February 3, 2020 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add markdownlint support #740

Add markdownlint support #740

adamconnelly commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

adamconnelly commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

tomkerkhove commented Oct 12, 2019

tomkerkhove commented Oct 12, 2019

adamconnelly commented Oct 12, 2019

adamconnelly commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

tomkerkhove commented Oct 14, 2019

Add markdownlint support #740

Add markdownlint support #740

Conversation

adamconnelly commented Oct 12, 2019

Proposed Changes

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

adamconnelly commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

tomkerkhove commented Oct 12, 2019

tomkerkhove commented Oct 12, 2019

adamconnelly commented Oct 12, 2019

adamconnelly commented Oct 12, 2019

promitor-bot commented Oct 12, 2019

tomkerkhove commented Oct 14, 2019