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

build: Update to use markdownlint compatible changelog #11733

Merged

Conversation

sudosubin
Copy link
Contributor

@sudosubin sudosubin commented Sep 3, 2023

Resolved markdownlint errors for CHANGELOG.md.

Motivation

Argo Workflows project has markdownlint configuration, but many markdown documents does not respect the lint rules. (I'm currently working on markdownlint.)

Modifications

I also updated ./hack/changelog.sh script.

Verification

$ markdownlint ./CHANGELOG.md
(clear)

Comment on lines +6 to +8
MD024:
siblings_only: true
MD034: false
Copy link
Member

Choose a reason for hiding this comment

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

Would this break the links to the existing headers, e.g. https://github.com/argoproj/argo-workflows/blob/master/CHANGELOG.md#v350-rc1-2023-08-15?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about the concerns, but the anchor to the header seems to work well: https://github.com/argoproj/argo-workflows/blob/428ddb118e826d322527a7a5c59e80418401ae1f/CHANGELOG.md#v350-rc1-2023-08-15

MD034 is a lint rule about using url in markdown file. (ex. https://example.com in *.md will produce warning)

Copy link
Member

@agilgur5 agilgur5 Sep 9, 2023

Choose a reason for hiding this comment

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

I think it would be better to configure these inline for the whole file rather than globally.

As in, using:

<!-- markdownlint-disable MD034 -->
<!-- markdownlint-configure-file { "MD024": { "siblings_only": true } } -->

Also some comments would be nice as to why these were disabled, as is convention and as the rest of the file does (otherwise, it's not clear why a rule is changed)

And uh.. I realized I've been looking at the wrong markdownlint this whole time 😭
that one doesn't support inline ignores but the one we are using actually does!

CHANGELOG.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove changes in this file? This will be auto-generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course! I force-pushed the latest commit. Thansk for the comment :D

Signed-off-by: sudosubin <sudosubin@gmail.com>
@sudosubin sudosubin force-pushed the feature/markdownlint-changelog branch from 428ddb1 to 1b9d36c Compare September 4, 2023 06:22
@terrytangyuan terrytangyuan changed the title feat: Update to use markdownlint compatible changelog build: Update to use markdownlint compatible changelog Sep 5, 2023
@terrytangyuan terrytangyuan enabled auto-merge (squash) September 5, 2023 15:36
@terrytangyuan terrytangyuan merged commit 5f5de6f into argoproj:master Sep 5, 2023
25 checks passed
@sudosubin sudosubin deleted the feature/markdownlint-changelog branch September 5, 2023 17:25
qudtjs0753 pushed a commit to qudtjs0753/argo-workflows that referenced this pull request Sep 6, 2023
@agilgur5 agilgur5 added the area/build Build or GithubAction/CI issues label Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants