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

[MINOR] Add PR description validation on documentation updates #10799

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

yihua
Copy link
Contributor

@yihua yihua commented Mar 2, 2024

Change Logs

As above, to make PR description validation strict.

No information in the "Documentation Update" section in the PR description will trigger PR validation failure:

Screenshot 2024-03-02 at 16 30 05

Impact

As above.

Risk level

none

Documentation Update

none

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@github-actions github-actions bot added the size:S PR with lines of changes in (10, 100] label Mar 2, 2024
@yihua yihua force-pushed the MINOR-add-docs-description-validation branch 3 times, most recently from c3ceee9 to 8ccbab2 Compare March 3, 2024 00:28
@yihua yihua force-pushed the MINOR-add-docs-description-validation branch from 8ccbab2 to 1982318 Compare March 3, 2024 00:29
@danny0405
Copy link
Contributor

Hmm, most of the PRs does not need update for doc, is it reasonable to by default do all the validations? And the [MINOR] xxx style title seems been fixed to pass the validation right?

@yihua
Copy link
Contributor Author

yihua commented Mar 3, 2024

Hmm, most of the PRs does not need update for doc, is it reasonable to by default do all the validations? And the [MINOR] xxx style title seems been fixed to pass the validation right?

User needs to just add N/A to the "Documentation Update" section in the PR description in this case (i.e., what this PR does). This is a reminder for author to check if there is any documentation update needed, not necessary to do update if the code changes are not user-facing.

@yihua
Copy link
Contributor Author

yihua commented Mar 4, 2024

@danny0405 I updated the template to be more instructive so user knows to put "N/A" if no docs update is needed.

scripts/pr_compliance.py Outdated Show resolved Hide resolved
Copy link
Contributor

@danny0405 danny0405 left a comment

Choose a reason for hiding this comment

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

+1, but I still think it is too strict for force update for the doc part.

@hudi-bot
Copy link

hudi-bot commented Mar 4, 2024

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@yihua
Copy link
Contributor Author

yihua commented Mar 4, 2024

+1, but I still think it is too strict for force update for the doc part.

We can experiment with this. This will be a good reminder for people to think about docs. We still have features of which the docs is missing.

@yihua
Copy link
Contributor Author

yihua commented Mar 4, 2024

"Java CI / validate-source", "Update Pr Compliance / run-tests", and "validate pr / validate-pr" pass which are suffucient for the tooling changes. There is no change to the production code. Merging this now.

@yihua yihua merged commit 8e63349 into apache:master Mar 4, 2024
42 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S PR with lines of changes in (10, 100]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants