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

Add markdown formatter for doc wrapping #1152

Merged
merged 2 commits into from
Mar 10, 2022
Merged

Add markdown formatter for doc wrapping #1152

merged 2 commits into from
Mar 10, 2022

Conversation

viniciusdc
Copy link
Contributor

Closes #1113

Changes introduced in this PR:

  • Add mdformat to wrap doc lines, explicitly added the configuration toml to further change in the default settings.
  • Set wrap line length to 180.

Types of changes

What types of changes does your PR introduce?

Put an x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

Requires testing

  • Yes
  • No

Further comments (optional)

Any wrapping will automatically happen when a .md file is committed.

@viniciusdc viniciusdc requested review from tonyfast and danlester March 8, 2022 19:08
@viniciusdc viniciusdc changed the title Add markdown linter for doc wrapping Add markdown formatter for doc wrapping Mar 8, 2022
Copy link
Contributor

@tonyfast tonyfast left a comment

Choose a reason for hiding this comment

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

seems reasonable to me. is 180 a lot of characters?

i wish that mdformat used the pyproject convention for configuration instead of adding another file. :(

Copy link
Member

@costrouc costrouc left a comment

Choose a reason for hiding this comment

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

@viniciusdc this looks good to me. Following this PR could you make a giant PR commit of strictly only the formatting changes?

@viniciusdc
Copy link
Contributor Author

seems reasonable to me. is 180 a lot of characters?

i wish that mdformat used the pyproject convention for configuration instead of adding another file. :(

Hi @tonyfast, indeed that would be great, they have an open PR to read from pyproject.toml, so maybe we can remove that extra file in the future. About the 180, I was following the original idea from this linked issue, and as there doesn't seem to be accordance about the best line length for documentation... having this as a maximum might be tolerable*.

I would prefer 88 as the limit to wrap, but I don't want this to be a restriction imposed further in the docs. So my way of thinking about this 180, is that it is "the max" length that needs to be respected.

@viniciusdc
Copy link
Contributor Author

@viniciusdc this looks good to me. Following this PR could you make a giant PR commit of strictly only the formatting changes?

Sure, I think it would be best to add those changes within this PR as its the source of the changes...

@viniciusdc viniciusdc force-pushed the add-markdownlint branch 2 times, most recently from 891bac8 to 1e8f847 Compare March 9, 2022 14:36
@viniciusdc
Copy link
Contributor Author

Sorry for the force-pushing, I tried to include the formatted docs, but creating a different PR for that definitely is the best path to go.

@viniciusdc viniciusdc added area: documentation 📖 Improvements or additions to documentation status: merge-ready labels Mar 9, 2022
@iameskild iameskild merged commit 68d01e0 into main Mar 10, 2022
@iameskild iameskild deleted the add-markdownlint branch March 10, 2022 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation 📖 Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH] - Impose documentation line length automatically
4 participants