Skip to content

[Improvement][Helm] using helm-docs to generate docs automatically #15299

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.

Sign up for GitHub

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

Jump to bottom
Merged
zhongjiajie merged 3 commits into from
Dec 29, 2023
Merged

[Improvement][Helm] using helm-docs to generate docs automatically #15299

zhongjiajie merged 3 commits into from
Dec 29, 2023

Conversation

@Gallardot
Copy link
Member

@Gallardot Gallardot commented Dec 9, 2023

Purpose of the pull request

Generate the chart's README.md file using helm-docs. When the values.yaml file is modified, the README.md file needs to be regenerated.

./mvnw validate -P helm-doc -pl :dolphinscheduler

CI validation has also been added.

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@Gallardot Gallardot force-pushed the helm-docs branch 2 times, most recently from 9e31946 to bf5d613 Compare December 9, 2023 17:42
@Gallardot
Copy link
Member Author

Gallardot commented Dec 9, 2023

I noticed that a deadline check CI has failed. However, I believe this error can be ignored because it's checking the newly added helm charts' README.md file.

@Gallardot Gallardot marked this pull request as ready for review December 9, 2023 17:56
@Gallardot
Copy link
Member Author

Gallardot commented Dec 9, 2023

I don't know why the OWASP Dependency Check has failed.

@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (93ea5f6) 38.11% compared to head (a71c399) 38.11%.

❗ Current head a71c399 differs from pull request most recent head 5c9e0a5. Consider uploading reports for the commit 5c9e0a5 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff            @@
##                dev   #15299   +/-   ##
=========================================
  Coverage     38.11%   38.11%           
- Complexity     4696     4697    +1     
=========================================
  Files          1300     1300           
  Lines         44778    44778           
  Branches       4800     4800           
=========================================
+ Hits          17067    17068    +1     
  Misses        25858    25858           
+ Partials       1853     1852    -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zhongjiajie
Copy link
Member

zhongjiajie commented Dec 12, 2023

I don't know why the OWASP Dependency Check has failed.

Cause we have security issue and we do not handle it, it is unrelated to you patch

@zhongjiajie
Copy link
Member

zhongjiajie commented Dec 12, 2023

I noticed that a deadline check CI has failed. However, I believe this error can be ignored because it's checking the newly added helm charts' README.md file.

Yeah, you are right, but we have to pass the docs check to merge the patch into the dev branch. So can we try to add disable for current line

[DolphinScheduler Helm Charts](https://github.com/apache/dolphinscheduler/blob/dev/deploy/kubernetes/dolphinscheduler/README.md) <!-- markdown-link-check-disable-line -->

@zhongjiajie
Copy link
Member

zhongjiajie commented Dec 12, 2023

others LGTM

@Gallardot
Copy link
Member Author

Gallardot commented Dec 12, 2023

I noticed that a deadline check CI has failed. However, I believe this error can be ignored because it's checking the newly added helm charts' README.md file.

Yeah, you are right, but we have to pass the docs check to merge the patch into the dev branch. So can we try to add disable for current line

[DolphinScheduler Helm Charts](https://github.com/apache/dolphinscheduler/blob/dev/deploy/kubernetes/dolphinscheduler/README.md) <!-- markdown-link-check-disable-line -->

Done. @zhongjiajie

ruanwenjun
ruanwenjun approved these changes Dec 21, 2023
View reviewed changes
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 28, 2023

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@zhongjiajie zhongjiajie merged commit 1c1d4bd into apache:dev Dec 29, 2023
@Gallardot Gallardot deleted the helm-docs branch December 29, 2023 03:26
@Gallardot Gallardot mentioned this pull request Dec 29, 2023
Merged
@zhongjiajie zhongjiajie added this to the 3.2.1 milestone Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ruanwenjun ruanwenjun ruanwenjun approved these changes

@zhongjiajie zhongjiajie Awaiting requested review from zhongjiajie

@EricGao888 EricGao888 Awaiting requested review from EricGao888 EricGao888 is a code owner

@caishunfeng caishunfeng Awaiting requested review from caishunfeng

@SbloodyS SbloodyS Awaiting requested review from SbloodyS SbloodyS is a code owner

Assignees

@Gallardot Gallardot

Labels

CI&CD document ready-to-merge

Projects

None yet

Milestone

3.2.1

Development

Successfully merging this pull request may close these issues.

[Improvement][Helm] using helm-docs to generate docs automatically

4 participants

@Gallardot @codecov-commenter @zhongjiajie @ruanwenjun