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.

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

Demo for Vale review comments in Prow CI #72450

Closed
wants to merge 2 commits into from
Closed

Demo for Vale review comments in Prow CI #72450

wants to merge 2 commits into from

Conversation

rohennes
Copy link
Contributor

@rohennes rohennes commented Mar 1, 2024 •
edited
Loading

Proof of concept (POC) for running Vale in the Prow CI. All feedback welcome.

  • Vale review runs on any modified or changed content only.
  • Runs on the latest commit every time you push.
  • Review comments are added for any error level (level: error) Vale rules.
  • It does not fail the build
  • Doesn't duplicate a comment if a comment already exists on the same line for the same issue
  • The Vale review check for the first commit in this PR took 16 seconds.

Test it out by PR'ing the prow-test-2 branch with content that will trigger an error level Vale rule. For example, use the term "hyperthreading" in content.

@openshift-ci openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 1, 2024
@rohennes rohennes changed the title Demo for Vale review comments Demo for Vale review comments in Prow CI Mar 1, 2024
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Mar 1, 2024 •
edited
Loading

🤖 Fri Mar 01 15:32:10 - Prow CI generated the docs preview:

https://72450--ocpdocs-pr.netlify.app/
https://72450--ocpdocs-pr.netlify.app/openshift-enterprise/latest/virt/post_installation_configuration/virt-post-install-network-config.html
https://72450--ocpdocs-pr.netlify.app/openshift-enterprise/latest/virt/storage/virt-configuring-local-storage-with-hpp.html

ocpdocs-previewbot
ocpdocs-previewbot reviewed Mar 1, 2024
View reviewed changes
modules/cnf-performing-end-to-end-tests-disconnected-mode.adoc
@@ -6,6 +6,8 @@
[id="cnf-performing-end-to-end-tests-disconnected-mode_{context}"]
= Running latency tests in a disconnected cluster

Adding the error term GIT to trigger a Vale alert.
Copy link

Choose a reason for hiding this comment

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

[error] RedHat.CaseSensitiveTerms: Use 'Git' rather than 'GIT'.

ocpdocs-previewbot
ocpdocs-previewbot reviewed Mar 1, 2024
View reviewed changes
modules/virt-creating-linux-bridge-nad-web.adoc

This is passive tense - the cat was eaten by the dog - but it is only a Vale suggestion so won't trigger a Vale review comment.

However, if I use the term white hat hacker, this term is a Vale error and triggers a Vale review comment.
Copy link

Choose a reason for hiding this comment

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

[error] RedHat.TermsErrors: Use 'offensive security researcher' rather than 'white hat hacker'.

ocpdocs-previewbot
ocpdocs-previewbot reviewed Mar 1, 2024
View reviewed changes
modules/virt-creating-storage-pool-pvc-template.adoc
@@ -46,7 +46,7 @@ spec:
----
<1> The `storagePools` stanza is an array that can contain both basic and PVC template storage pools.
<2> Specify the storage pool directories under this node path.
<3> Optional: The `volumeMode` parameter can be either `Block` or `Filesystem` as long as it matches the provisioned volume format. If no value is specified, the default is `Filesystem`. If the `volumeMode` is `Block`, the mounting pod creates an XFS file system on the block volume before mounting it.
<3> Optional: The `volumeMode` parameter can be either `Block` or `Filesystem` as long as it matches the provisioned volume format. If no value is specified, the default is `Filesystem`. If the `volumeMode` is `Block`, the mounting pod creates an XFS file system on the block volume before mounting it. Editing this sentence triggers a Vale review comment.
Copy link

Choose a reason for hiding this comment

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

[error] RedHat.TermsErrors: Use 'if' or 'provided that' rather than 'as long as'.

ocpdocs-previewbot
ocpdocs-previewbot reviewed Mar 1, 2024
View reviewed changes
modules/cnf-performing-end-to-end-tests-disconnected-mode.adoc
@@ -6,6 +6,10 @@
[id="cnf-performing-end-to-end-tests-disconnected-mode_{context}"]
= Running latency tests in a disconnected cluster

Adding the error term GIT to trigger a Vale alert.

Adding this in a second commit to show only 1 further Vale review comment: lower case podman is an error.
Copy link

Choose a reason for hiding this comment

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

[error] RedHat.CaseSensitiveTerms: Use 'Podman' rather than 'podman'.

Copy link
Contributor

Choose a reason for hiding this comment

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

1 further Vale review comment

Is it possible to point out where numeral should be spelled out? ISG: In general write in words numbers that are less than 10.

Copy link
Contributor Author

@rohennes rohennes Mar 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi Michael, this would be a potential Vale rule. If it the rule was created with a level of "error", then yes it would trigger a review comment. Good idea. The only tricky part is avoiding false positives. I opened a Vale issue. Thanks!

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Apr 16, 2024

@rohennes: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 16, 2024
@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 15, 2024
@openshift-bot
Copy link

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci bot closed this Sep 15, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Sep 15, 2024

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mburke5678 mburke5678 mburke5678 left review comments

@ocpdocs-previewbot ocpdocs-previewbot ocpdocs-previewbot left review comments

Assignees
No one assigned
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rohennes @ocpdocs-previewbot @openshift-bot @mburke5678