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

feat(CI):enable markdownlint and typos in docs.yml #508

Merged
merged 11 commits into from
Aug 12, 2024

Conversation

ywh555hhh
Copy link
Contributor

Improve CI flow, for docs.yml, enable markdownlint and typos

Reason for this PR

to fix issue #499

What changes are included in this PR?

enable markdownlint and typos in docs.yml

Are these changes tested?

no local test

Are there any user-facing changes?

no

markdownlint and typos
@SemyonSinchenko
Copy link
Member

May we also make a script for do that before commit? For example, in folder ./dev something like fix_spelling.sh?

@ywh555hhh
Copy link
Contributor Author

I'll try to fix it automatically with markdownlint

@ywh555hhh
Copy link
Contributor Author

When using Markdownlint for document formatting checks, I encountered some issues that could not be automatically resolved, mainly involving a large number of MD013 (line length is too long) and MD033 (inline HTML) issues, as well as one MD025 (multiple top-level headings) issue.

For example, I encountered MD013 and MD033 issues on line 110 of the specification/format.md file, and an MD025 issue in the libraries/cpp/getting-started.md file.

To resolve these issues, I have configured Markdownlint to ignore these three types of issues in the .markdownlint.yaml configuration file. The specific configuration is as follows:

# Ignore MD013, as the document requires longer lines to maintain the integrity of code examples
MD013: false

# Ignore MD033, as inline HTML is necessary in some cases, such as specific formatting requirements
MD033: false

# Ignore MD025, as the document structure requires multiple top-level headings to reflect different sections or parts
MD025: false

I believe this is a reasonable solution because these issues are mainly caused by the mismatch between our document format and the default rules of Markdownlint. By customizing our Markdownlint configuration, we can ensure that it better adapts to our document structure and formatting needs.

I have utilized the markdownlint and typos tools to automatically rectify some issues in the .md files within the doc directory. For issues that could not be automatically fixed, I have chosen to ignore them by setting rules in the .markdownlint.yaml file.
Copy link
Contributor

@acezen acezen left a comment

Choose a reason for hiding this comment

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

Hi, @ywh555hhh, it seems that the markdown format work, but there still some format error in CI, could you help to fix them? then the PR can be merged.

@@ -0,0 +1,8 @@
# 忽略 MD013,因为文档需要较长的行以保持代码示例的完整性
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to use english as document language

@ywh555hhh
Copy link
Contributor Author

Hi, @ywh555hhh, it seems that the markdown format work, but there still some format error in CI, could you help to fix them? then the PR can be merged.

I expect to have time to deal with it next week

@yecol
Copy link
Contributor

yecol commented Aug 6, 2024

Cool ~ Enhanced CI to improve the quality of our documentations! LGTM. :)

Copy link
Contributor

@acezen acezen left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @ywh555hhh

@acezen
Copy link
Contributor

acezen commented Aug 7, 2024

Hi, @ywh555hhh, I have a question, why this change affect to the testing submodule?

Copy link
Contributor

@acezen acezen left a comment

Choose a reason for hiding this comment

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

the testing submodule should not be modified

@ywh555hhh
Copy link
Contributor Author

Hi, @ywh555hhh, I have a question, why this change affect to the testing submodule?

Seems I ran some tool locally incorrectly

@ywh555hhh
Copy link
Contributor Author

the testing submodule should not be modified

testing submodule is no longer affected

Copy link
Contributor

@acezen acezen left a comment

Choose a reason for hiding this comment

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

LGTM, thanks very much for the contribution!

@acezen acezen merged commit 8bb741e into apache:main Aug 12, 2024
2 checks passed
acezen pushed a commit to Elssky/incubator-graphar that referenced this pull request Aug 27, 2024
Elssky pushed a commit to Elssky/incubator-graphar that referenced this pull request Sep 23, 2024
Elssky pushed a commit to Elssky/incubator-graphar that referenced this pull request Oct 8, 2024
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.

4 participants