thephpleague / csv

CSV data manipulation made easy in PHP
https://csv.thephpleague.com
MIT License
3.33k stars 333 forks source link

Add markdownlint #441

Closed pdelre closed 2 years ago

pdelre commented 2 years ago

RE: #437

This adds the .markdownlint.yml configuration from thephpleague/commonmark. I've run the configuration through a smaller subset of the docs/ directory as a sample of the likely changes in the larger directories (docs/7.0/, docs/8.0/, & docs/9.0/).

The most common violation is the code fense style (``</code> vs~~~`). Backticks were preferred for thephpleague/commonmark, but if this project would rather use tilde, I will adjust the configuration.

If this sample looks good, please let me know if you'd prefer one large PR or several medium sized PRs. Below are a breakdown of the remaining violations that can be addressed.

docs/9.0/

$ markdownlint 9.0 2>&1 | awk '{print $2}' | sort | uniq -c | sort -r
 188 MD048/code-fence-style
  13 MD012/no-multiple-blanks
  13 MD010/no-hard-tabs
  10 MD004/ul-style
   9 MD024/no-duplicate-heading/no-duplicate-header
   5 MD009/no-trailing-spaces
   3 MD047/single-trailing-newline
   2 MD040/fenced-code-language
   2 MD031/blanks-around-fences
   2 MD007/ul-indent
   1 MD046/code-block-style
   1 MD038/no-space-in-code
   1 MD026/no-trailing-punctuation
   1 MD014/commands-show-output
   1 MD001/heading-increment/header-increment

docs/8.0/

$ markdownlint 8.0 2>&1 | awk '{print $2}' | sort | uniq -c | sort -r
 128 MD048/code-fence-style
  47 MD004/ul-style
  35 MD010/no-hard-tabs
  18 MD024/no-duplicate-heading/no-duplicate-header
   8 MD012/no-multiple-blanks
   7 MD007/ul-indent
   2 MD047/single-trailing-newline
   2 MD040/fenced-code-language
   2 MD026/no-trailing-punctuation
   2 MD014/commands-show-output

docs/7.0/

$ markdownlint 7.0 2>&1 | awk '{print $2}' | sort | uniq -c | sort -r
  71 MD048/code-fence-style
  64 MD004/ul-style
  29 MD010/no-hard-tabs
   9 MD007/ul-indent
   5 MD031/blanks-around-fences
   2 MD047/single-trailing-newline
   2 MD040/fenced-code-language
   2 MD014/commands-show-output
   2 MD012/no-multiple-blanks
   1 MD036/no-emphasis-as-heading/no-emphasis-as-header
   1 MD026/no-trailing-punctuation
nyamsprod commented 2 years ago

it's all ok for me you can go with the current settings ;)

pdelre commented 2 years ago

@nyamsprod I've fixed the various violations from the original description, with a couple of exceptions.

  1. I enabled duplicate headers under different nestings. This was found when there were multiple H1/H2 headings that had commonly reused H3/H4 headers, typically named Example, Description, etc. The alternative would be to format those H3/H4 headers with bold (**Example**) styling but the consistent pattern in this project is to use headers.
  2. I removed any PHP open tags (<?php) from the start of code blocks. I feel these are an anti-pattern for PHP code blocks as open tags are to be expected to be common knowledge to the reader. When looking at the usage in this specific project, there also seemed to be an implied trend towards not including them in examples as they mostly existed in the 7.0/ & 8.0/ sections. This is also consistent with thephpleague/commonmark's documentation.
Version Examples with <?php Examples without <?php
7.0 138 0
8.0 250 2
9.0 12 360

I'd also ask if you found this valuable, could you please add the hacktoberfest label to the project so they can count this towards my profile? More from this year's documentation.

nyamsprod commented 2 years ago

I would prefer if you split the PR on different mid sized: 1 PR per documentation version 1 PR for the "rest" CHANGELOG and README for instance can be together and then I will merge them in order

pdelre commented 2 years ago

👍 I should be free to do that today!

On Fri, Oct 15, 2021, 5:54 AM ignace nyamagana butera < @.***> wrote:

I would prefer if you split the PR on different mid sized: 1 PR per documentation version 1 PR for the "rest" CHANGELOG and README for instance can be together and then I will merge them in order

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thephpleague/csv/pull/441#issuecomment-944160708, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQXMB5JRZNFENGEPNTNALUG727DANCNFSM5FT2SVBQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

pdelre commented 2 years ago

@nyamsprod I've created https://github.com/thephpleague/csv/pull/442 as the initial PR with the configuration and the smaller group of markdown fixes. Once it is merged, I can create the PRs for each of the versioned directories in docs/.

I don't think I can create them a head of time as the base branch only exists on my fork.

nyamsprod commented 2 years ago

All is merged can we then close this PR then ?

pdelre commented 2 years ago

Yes 🎉