Skip to content

Make sure emphasis in UPDATING in .md is consistent#21804

Merged
potiuk merged 1 commit intoapache:mainfrom
potiuk:make-sure-emphasis-is-consistent
Feb 24, 2022
Merged

Make sure emphasis in UPDATING in .md is consistent#21804
potiuk merged 1 commit intoapache:mainfrom
potiuk:make-sure-emphasis-is-consistent

Conversation

@potiuk
Copy link
Copy Markdown
Member

@potiuk potiuk commented Feb 24, 2022

There is a new rule in markdownlint which has been violated in
main when new version of pre-commits is installed introduced in
the #21734


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

There is a new rule in markdownlint which has been violated in
main when new version of pre-commits is installed introduced in
the apache#21734
@ashb
Copy link
Copy Markdown
Member

ashb commented Feb 24, 2022

Hmm I've already used underscore for emphasis/italics

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Feb 24, 2022
@github-actions
Copy link
Copy Markdown

The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Feb 24, 2022

It's just it was inconsistently used in this file - some emphasis used _ and some * :) (we had both in the file).
Seems like a new rule added recently in markdownlint.

@potiuk potiuk merged commit a724523 into apache:main Feb 24, 2022
@potiuk potiuk deleted the make-sure-emphasis-is-consistent branch February 24, 2022 19:36
@ashb
Copy link
Copy Markdown
Member

ashb commented Feb 24, 2022

Did we mean our version of markdownlint to change without explicit action?

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Feb 24, 2022

Did we mean our version of markdownlint to change without explicit action?

Nope. I was just looking at it why it did not install for me (but I wanted to quickly fix it first)

It is installed automatically by pre-commit - we have:

      - id: markdownlint
        name: Run markdownlint
        description: Checks the style of Markdown files.
        entry: markdownlint
        language: node
        types: [markdown]
        files: \.(md|mdown|markdown)$
        additional_dependencies: ['markdownlint-cli']

And markdownlint-cli - drags markdownlint alongside: https://www.npmjs.com/package/markdownlint. Markdownlint was released on Jan 15 but if you had pre-commit installed locally, it did not upgrade to latest version.

Apparenttly when you already had markdownlint-cli (thus markdownlint) installed by pre-commit, it would not trigger automated update to latest version. I am fixing this now.

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Feb 24, 2022

This is really strange - I even can't reinstall markdownlint from scratch now (TypeError: files.flat is not a function)....
This is rather bizarre.

rustikk pushed a commit to rustikk/airflow that referenced this pull request Feb 25, 2022
There is a new rule in markdownlint which has been violated in
main when new version of pre-commits is installed introduced in
the apache#21734
@jedcunningham jedcunningham added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Feb 28, 2022
@potiuk potiuk restored the make-sure-emphasis-is-consistent branch April 26, 2022 20:48
@potiuk potiuk deleted the make-sure-emphasis-is-consistent branch July 29, 2022 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) okay to merge It's ok to merge this PR as it does not require more tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants