Skip to content

Add markdown lint github action and Apply lint#93

Closed
st-Wook wants to merge 51 commits intoGDevelopApp:mainfrom
st-Wook:mark_down_lint
Closed

Add markdown lint github action and Apply lint#93
st-Wook wants to merge 51 commits intoGDevelopApp:mainfrom
st-Wook:mark_down_lint

Conversation

@st-Wook
Copy link
Copy Markdown
Contributor

@st-Wook st-Wook commented Jun 12, 2023

Could you share your thoughts on adding a Markdown lint Github action to more easily find formatting errors in Markdown-formatted documents, especially things like nested lists? If you think it's a good idea, please decide on the following format rules (and let me know if you have any additional thoughts.):

  1. Should the list indentation be 0 or 2?
  2. Should we allow multiple consecutive blank lines?
  3. Should we enforce blanks around headers?

I would also like to ask for our opinion on converting to Markdown lint rules when generating extended description documents!

lint Rules

@vercel
Copy link
Copy Markdown

vercel bot commented Jun 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
gdevelop-wiki ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 17, 2023 9:00am

@Bouh
Copy link
Copy Markdown
Member

Bouh commented Jun 14, 2023

It's just my trough:

  • Should the list indentation be 0 or 2? 2 spaces
  • Should we allow multiple consecutive blank lines? No
  • Should we enforce blanks around headers? Yes

@st-Wook
Copy link
Copy Markdown
Contributor Author

st-Wook commented Jun 14, 2023

It seems the Lint checker only verifies the indent for Unordered Lists. The indent for Ordered Lists should also be two, right?

@AlexandreSi
Copy link
Copy Markdown
Contributor

Should the list indentation be 0 or 2? 2 spaces

I would say 0: I don't want to add 2 spaces at the beginning of each bullet point for a single-level list. Why are you in favor of 2 spaces @Bouh ?

It seems the Lint checker only verifies the indent for Unordered Lists. The indent for Ordered Lists should also be two, right?

I would say yes.


Regarding those formatting rules, I'm afraid it will prevent users to suggest documentation changes. Let me explain:

  • Not everyone is a markdown expert;
  • This version of markdown is a bit demanding on some aspects;
  • Not everyone has an IDE with an autoformat action.

Considering those 3 points, I'm afraid 85% of the users opening a PR in the repo will have a failing CI and get discouraged by it and end up closed.

Or this check could be converted in a "Fix action" that adds a commit with the fixes?

@st-Wook
Copy link
Copy Markdown
Contributor Author

st-Wook commented Jun 23, 2023

Or this check could be converted in a "Fix action" that adds a commit with the fixes?

I have made the modifications as you suggested! However, the GitHub action is failing due to permission issues. I have conducted the tests on my local repo PR! (local pr link)

@st-Wook
Copy link
Copy Markdown
Contributor Author

st-Wook commented Jun 23, 2023

In order to have a bot automatically make commits and add comments to PR created from forked branches, a GitHub Token with write permissions is required...

Edit: I think "pull_request_target" would have solved the permission issue.

@st-Wook
Copy link
Copy Markdown
Contributor Author

st-Wook commented Jul 17, 2023

Creating commits to automatically fix the Lint seems challenging due to various issues... I'll open a new PR when a solution is found!

@st-Wook st-Wook closed this Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants