You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2023/01/18 15:03:16 UTC

[GitHub] [beam] damccorm commented on a diff in pull request #25048: Don't validate trailing whitespace for *.md files.

damccorm commented on code in PR #25048:
URL: https://github.com/apache/beam/pull/25048#discussion_r1073643348


##########
sdks/python/test-suites/tox/py38/build.gradle:
##########
@@ -120,7 +120,6 @@ task archiveFilesToLint(type: Zip) {
   destinationDirectory = file("$buildDir/dist")
 
   from ("$rootProject.projectDir") {
-    include "**/*.md"

Review Comment:
   Most style guides seem to recommend against this as well (I only sampled 3, but they all agree):
   - https://arcticicestudio.github.io/styleguide-markdown/rules/whitespace.html
   - https://www.markdownguide.org/basic-syntax/#line-break-best-practices
   - https://google.github.io/styleguide/docguide/style.html#trailing-whitespace
   
   I put up #25055 as an alternative



##########
sdks/python/test-suites/tox/py38/build.gradle:
##########
@@ -120,7 +120,6 @@ task archiveFilesToLint(type: Zip) {
   destinationDirectory = file("$buildDir/dist")
 
   from ("$rootProject.projectDir") {
-    include "**/*.md"

Review Comment:
   I'd vote we keep this rule, especially for markdown. While trailing whitespace in markdown is meaningful, its also an antipattern IMO - its an invisible change to reviewers that changes the rendering of the document in a meaningful way, and many IDEs will remove it automatically on save. A better way to do this IMO is to insert an empty line or a break tag.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org