-
Notifications
You must be signed in to change notification settings - Fork 4.8k
chore(workflows): Consolidate Vale linting into Reviewdog workflow #67620
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
chore(workflows): Consolidate Vale linting into Reviewdog workflow #67620
Conversation
Original prompt from [email protected]
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Helpful Resources
PR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
|
Deploy preview for airbyte-docs ready! ✅ Preview Built with commit d088db3. |
… comments Co-Authored-By: [email protected] <[email protected]>
Good catch! I investigated the Vale action documentation and found that we can use the I've now:
This way, developers running Vale locally will see all suggestions, while the CI workflow will only report warnings and errors in PR reviews. |
Good question! I matched the version used in the existing I've updated it to use the V5 hash ( |
Done! I've added inline comments explaining the purpose of each of the 5 Vale configuration parameters:
|
Great. Now that we have multiple checkouts happening, is it possible to consolidate into one checkout that all jobs can use? This seems more efficient especially if we later add new jobs. |
Good question! In GitHub Actions, jobs run on separate runners and don't share workspaces by default. I can think of a few approaches:
Which approach would you prefer? For most workflows like this, option 1 (same version) or option 3 (keep as-is) are typical. Let me know what you had in mind! |
Alright this looks fine, we'll stick with using distinct checkouts on each runner. Now let's run a test. Create a new test PR and attempt to merge it back into this one. In that PR, create a new file that violates linting rules. You can populate it with the content from docs/platform/community/getting-support.md, because that file has a variety of violations that should trigger the linter. You should also mess the test file up a bit by removing blank lines around headings and lists. This should provide a long list of violations from both markdownlint and vale that shows this workflow in action. |
✅ Test Results: Reviewdog Workflow ValidatedI've created test PR #67621 to validate the consolidated Reviewdog workflow, and it's working perfectly! Test PR: #67621 Results:
The test file intentionally violated formatting rules by removing blank lines around headings and lists, and included content that triggers Vale style violations. Both linters caught and reported the issues correctly. The consolidated Reviewdog workflow is ready for use! 🎉 |
Great. Last step is to update contribution documentation. https://docs.airbyte.com/platform/next/contributing-to-airbyte/writing-docs#linting
|
…workflow - Remove outdated note about MarkdownLint being the only linter on PRs - Add new 'Linters in CI' subsection explaining how both linters work in CI - Apply changes to current and historical platform versions (1.6, 1.7, 1.8) - CI now runs both Vale (warnings/errors) and MarkdownLint on PRs - Suggestions only available when running linters locally Co-Authored-By: [email protected] <[email protected]>
Change 'sections above' to 'sections below' since the linting instructions appear after the 'Linters in CI' subsection, not before it. Co-Authored-By: [email protected] <[email protected]>
What
Consolidates the Vale linting job from PR #65973 into the existing Reviewdog workflow. This adds Vale as a second documentation linting job alongside MarkdownLint, both using the same reviewdog reporter for consistency.
Related to #65973
How
Workflow Changes (
.github/workflows/reviewdog.yml
):vale
job that runs after themarkdownlint
jobgithub-pr-review
reporter (matching MarkdownLint's configuration)filter_mode: added
to only lint newly added contentfail_on_error: false
to keep Vale checks non-blockingVale Configuration Changes (
docusaurus/vale.ini
):MinAlertLevel
fromsuggestion
towarning
Review guide
.github/workflows/reviewdog.yml
- Review the Vale job configuration:errata-ai/vale-action
parameters are correctgithub-pr-review
is the appropriate reporter (changed fromgithub-pr-check
in the original PR Docs Vale lint on PRs #65973)filter_mode: added
andfail_on_error: false
are the desired settingsdocusaurus/vale.ini
- Review the MinAlertLevel change:MinAlertLevel = warning
is the desired behavior (excludes suggestions, only shows warnings and errors)airbyte.punctuation = NO
rule was already removed in master, so no changes needed thereUser Impact
Positive:
Neutral:
Can this PR be safely reverted and rolled back?
This only adds a new linting job to the Reviewdog workflow. Reverting would simply remove the Vale checks without affecting the existing MarkdownLint job or any other workflows.
Session Details:
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.