Skip to content

[Merged by Bors] - Cleanup of Markdown Files and add CI Checking#1463

Closed
MinerSebas wants to merge 18 commits intobevyengine:mainfrom
MinerSebas:Markdown_fixes
Closed

[Merged by Bors] - Cleanup of Markdown Files and add CI Checking#1463
MinerSebas wants to merge 18 commits intobevyengine:mainfrom
MinerSebas:Markdown_fixes

Conversation

@MinerSebas
Copy link
Contributor

@MinerSebas MinerSebas commented Feb 17, 2021

I have run the VSCode Extension markdownlint on all Markdown Files in the Repo.
The provided Rules are documented here: https://github.com/DavidAnson/markdownlint/blob/v0.23.1/doc/Rules.md

Rules I didn't follow/fix:

  • MD024/no-duplicate-heading
    • Changelog: Here Heading will always repeat.
    • Examples Readme: Platform-specific documentation should be symmetrical.
  • MD025/single-title
  • MD026/no-trailing-punctuation
    • Caused by the ! in "Hello, World!".
  • MD033/no-inline-html
    • The plugins_guidlines file does need HTML, so the shown badges aren't downscaled too much.
  • MD036/no-emphasis-as-heading:
    • This Warning only Appears in the Github Issue Templates and can be ignored.
  • MD041/first-line-heading
    • Only appears in the Readme for the AlienCake example Assets, which is unimportant.

I also sorted the Examples in the Readme and Cargo.toml in this order/Priority:

  • Topic/Folder
  • Introductionary Examples
  • Alphabetical Order

The explanation for each case, where it isn't Alphabetical :

  • Diagnostics
    • log_diagnostics: The usage of inbuild Diagnostics is more important than creating your own.
  • ECS (Entity Component System)
    • ecs_guide: The guide should be read, before diving into other Features.
  • Reflection
    • reflection: Basic Explanation should be read, before more advanced Topics.
  • WASM Examples
    • hello_wasm: It's "Hello, World!".

@Ratysz Ratysz added the C-Docs An addition or correction to our documentation label Feb 17, 2021
@alice-i-cecile alice-i-cecile added the A-Build-System Related to build systems or continuous integration label Feb 17, 2021
@alice-i-cecile
Copy link
Member

It would be nice to add this to the CI as well somehow, to avoid the churn of making commits like this periodically.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 17, 2021
@MinerSebas
Copy link
Contributor Author

MinerSebas commented Feb 17, 2021

I have added super-linter Github Action to check the Files.
This Action provides many different Linters, but for now, I only activated the Markdown Linter.

Other Linters that could be applicable here are YAML and Bash checks.
The HTML and Copy/paste detection do currently provided false positive Warnings.
A complete Report of the Repository can be seen here

As for the previously ignored Markdown Rules, they are now either fixed or explicitly marked as to ignore.

Edit:
I say the HTML check is a false positive because we have only 1. HTML file in the Repo, which is in the WASM Examples Folder.
The provided errors could be fixed, but I think it would be unnecessary to activate the check for 1. File.

@MinerSebas MinerSebas changed the title Cleanup of Markdown Files Cleanup of Markdown Files and add CI Checking Feb 17, 2021
Copy link
Member

@CleanCut CleanCut left a comment

Choose a reason for hiding this comment

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

Nice work. This looks like a good step in getting and keeping the markdown files clean. I especially like the random typos you caught and fixed in the process.

I have some suggestions I think would improve this even more, mostly based on my experience with GitHub Actions for CI. However, I'm not the gatekeeper and defer to @cart's preferences. 😄

@MinerSebas
Copy link
Contributor Author

I added Documentation for the used Linters in the new ./docs/linters.md File.

The description for Clippy and rustfmt are lackluster because I was unsure what else to write, but I am open for suggestions.

Copy link
Member

@CleanCut CleanCut left a comment

Choose a reason for hiding this comment

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

I have some grammar/wording suggestions, but this looks great to me. Well done. 😄

Note to self: If this PR is merged first, I'll need to update docs/linters.md in #1387

Base automatically changed from master to main February 19, 2021 20:44
@cart
Copy link
Member

cart commented Feb 22, 2021

This looks good to me. I'm not the biggest fan of forcing "semantically correct" header types (# header comes first, then ## header, etc) over the "using header type for preferred style" approach we are currently using. But adding some structure to our markdown is a good idea and thats a pretty small thing to give up (and we can always fall back to html if we really need a particular style).

@cart
Copy link
Member

cart commented Feb 22, 2021

bors r+

bors bot pushed a commit that referenced this pull request Feb 22, 2021
I have run the VSCode Extension [markdownlint](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) on all Markdown Files in the Repo.
The provided Rules are documented here: https://github.com/DavidAnson/markdownlint/blob/v0.23.1/doc/Rules.md

Rules I didn't follow/fix:
* MD024/no-duplicate-heading
  * Changelog: Here Heading will always repeat.
  * Examples Readme: Platform-specific documentation should be symmetrical.
* MD025/single-title
* MD026/no-trailing-punctuation
  * Caused by the ! in "Hello, World!".
* MD033/no-inline-html
  * The plugins_guidlines file does need HTML, so the shown badges aren't downscaled too much.
* ~~MD036/no-emphasis-as-heading:~~
  * ~~This Warning only Appears in the Github Issue Templates and can be ignored.~~
* ~~MD041/first-line-heading~~
  * ~~Only appears in the Readme for the AlienCake example Assets, which is unimportant.~~

---

I also sorted the Examples in the Readme and Cargo.toml in this order/Priority:
* Topic/Folder
* Introductionary Examples
* Alphabetical Order

The explanation for each case, where it isn't Alphabetical :
* Diagnostics
  * log_diagnostics: The usage of inbuild Diagnostics is more important than creating your own.
* ECS (Entity Component System)
  * ecs_guide: The guide should be read, before diving into other Features.
* Reflection
  * reflection: Basic Explanation should be read, before more advanced Topics.
* WASM Examples
  * hello_wasm: It's "Hello, World!".
@bors
Copy link
Contributor

bors bot commented Feb 22, 2021

@bors bors bot changed the title Cleanup of Markdown Files and add CI Checking [Merged by Bors] - Cleanup of Markdown Files and add CI Checking Feb 22, 2021
@bors bors bot closed this Feb 22, 2021
@MinerSebas MinerSebas deleted the Markdown_fixes branch February 23, 2021 10:29
bors bot pushed a commit that referenced this pull request Apr 12, 2021
Update `linters.md` with info about `cargo ci` xtask as per #1463 (review)

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
jihiggins pushed a commit to jihiggins/bevy that referenced this pull request Apr 18, 2021
Update `linters.md` with info about `cargo ci` xtask as per bevyengine#1463 (review)

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
Update `linters.md` with info about `cargo ci` xtask as per bevyengine#1463 (review)

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Build-System Related to build systems or continuous integration C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants