build: have vale lint only run on affected files #30378

maxandersen · 2023-01-14T08:35:34Z

attempt on have vale lint only run on added/modified files.

for now just a draft as it passes all changed files to vale not just .adoc files.

quarkus-bot · 2023-01-14T08:35:36Z

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

maxandersen · 2023-01-14T08:53:47Z

actually, looks like vale automatically only scans files that matches an extension it knows about adoc, markdown, etc.

thus not sure we even need to filter the incoming list?

maxandersen · 2023-01-14T08:55:25Z

my only concern is why propertie files are set to be identified as markdown files?

that could generate warning noise. Any reason why vale would want to look at properties? (there are none in our docs sources atm)

famod · 2023-01-14T22:32:41Z

.github/workflows/vale.yml

@@ -22,13 +22,17 @@ jobs:
    steps:
      - name: Checkout
        uses: actions/checkout@v3
+      - id: files
+        uses: jitterbit/get-changed-files@v1


I don't have any alternative to offer right now, but jitterbit/get-changed-files#55 and the lack of recent activity in that repo might be issues worth considering.

@maxandersen &@famod -

On my sandbox, I tested the exact Vale configuration that is suggested in this PR and added yaml gradle dev ..bad stuff to catch vale! to line 58 in one file doc-contribute-docs-howto.adoc.

The jitterbit et al config updates in this PR work perfectly afaics! The Vale Job in the same PR containing 1 edited adoc file now only takes 2 seconds instead of 1minute and 5 seconds!! Super job 👍 Thank you for your help to tweak this.

I also submitted PR #30397 to target main with some tweaks to rules and the vale.ini as per the discussion above. Most of the config is optional and was specific to another project that checked .rst, properties, markdown and other doc source formats.

I'd suggest using a fork of that action which doesn't use the deprecated set-output command: jitterbit/get-changed-files#55 (comment)
(at least until the original action shows signs of improvement)

@famod & @maxandersen - FYI - The Vale GitHub action issue was just updated to report that a fix "should be out next week." The same contributor is also working on a performance fix relating to Vale unnecessarily running all files when the config is set to report only problems with the diff. 🤞 # 82.

@michelle-purcell got a link to that issue? if the vale github action is being updated we might just wait for that...

@maxandersen - Here are the 2 issues:

89 - Action fails even if fail_on_error is false

and
82 - reviewdog Vale is slower compared to its ancestor

okey so the fix they plan on next week is to just having the fail on error issue.

so i would say we merge what we have to getthe doc check to be much faster.

famod · 2023-01-14T22:33:24Z

Any reason why vale would want to look at properties?

Resource bundles with localized strings maybe?

maxandersen · 2023-01-15T12:51:37Z

Sure; but since we don't have those...could we remove that line and be done with it?

michelle-purcell · 2023-01-16T12:37:00Z

my only concern is why propertie files are set to be identified as markdown files?

that could generate warning noise. Any reason why vale would want to look at properties? (there are none in our docs sources atm)

Hey @maxandersen

This is an optional mapping in the .vale.ini config file, which the Quarkus vale.ini inherited when copied over from the vale-at-red-hat repo. According to the Vale community docs, it's optional so Vale doesn't need it. I posted a Q in the vale-at-red-hat community slack channel to be sure there are no particular use cases/scenarios. (@rolfedh - are you aware of any?)

As we only want to check AsciiDoc files in one specific directory, I think we could also look at removing other optional config lines in https://github.com/quarkusio/quarkus/blob/main/docs/.vale/vale.ini.

I'll do some tests on my sandbox with the minimum config + Quarks rules and will submit a PR with those tweaks.

Thanks

michelle-purcell · 2023-01-16T18:49:47Z

docs/src/main/asciidoc/doc-contribute-docs-howto.adoc

@@ -49,6 +49,8 @@ As you make changes, you can rebuild the `docs` module specifically to update th
 ./mvnw -f docs clean install
 ----

+yaml gradle dev ..bad stuff to catch vale!
+


The 2 spelling warnings above for 'gradle' and 'yaml' are not correct. The spelling is OK but the CASE is not. I fixed these in PR #30397. With this fix you only get one Vale warning for 'gradle' and 'yaml' .

I just added those for test of the worflow. I'll get the pr updated to not include this before merging.

@maxandersen - Understood but the Vale rule results that were flagged on those terms didn't look good and needed some rule tweaks.

michelle-purcell

Great improvement to the Vale linter job. Thank you ⭐

quarkus-bot · 2023-05-03T03:19:19Z

Failing Jobs - Building `670bf99`

Status	Name	Step	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Failures	Logs	Raw logs
✖	JVM Tests - JDK 17	`Build`	Failures	Logs	Raw logs
⌛	JVM Tests - JDK 19	`Build`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: extensions/jdbc/jdbc-oracle/deployment 
! Skipped: extensions/reactive-oracle-client/deployment integration-tests/jpa-oracle integration-tests/opentelemetry-jdbc-instrumentation and 1 more

📦 extensions/jdbc/jdbc-oracle/deployment

✖ io.quarkus.jdbc.oracle.deployment.DevServicesOracleDatasourceTestCase. - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.datasource.deployment.devservices.DevServicesDatasourceProcessor#launchDatabases threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/gvenzl/oracle-free:23-slim-faststart

⚙️ JVM Tests - JDK 17 #

- Failing: extensions/jdbc/jdbc-oracle/deployment 
! Skipped: extensions/reactive-oracle-client/deployment integration-tests/jpa-oracle integration-tests/opentelemetry-jdbc-instrumentation and 1 more

📦 extensions/jdbc/jdbc-oracle/deployment

✖ io.quarkus.jdbc.oracle.deployment.DevServicesOracleDatasourceTestCase. - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.datasource.deployment.devservices.DevServicesDatasourceProcessor#launchDatabases threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/gvenzl/oracle-free:23-slim-faststart

⚙️ JVM Tests - JDK 19 #

📦 extensions/jdbc/jdbc-oracle/deployment

✖ io.quarkus.jdbc.oracle.deployment.DevServicesOracleDatasourceTestCase. - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.datasource.deployment.devservices.DevServicesDatasourceProcessor#launchDatabases threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/gvenzl/oracle-free:23-slim-faststart

quarkus-bot bot added area/documentation area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure labels Jan 14, 2023

maxandersen force-pushed the slimmervale branch 2 times, most recently from 296532e to cba55ea Compare January 14, 2023 08:42

maxandersen marked this pull request as ready for review January 14, 2023 08:52

This comment has been minimized.

Sign in to view

famod reviewed Jan 14, 2023

View reviewed changes

michelle-purcell mentioned this pull request Jan 16, 2023

Tweak and optimize vale config #30397

Merged

michelle-purcell reviewed Jan 16, 2023

View reviewed changes

michelle-purcell approved these changes Jan 16, 2023

View reviewed changes

maxandersen force-pushed the slimmervale branch from cba55ea to 1bc4ff1 Compare January 17, 2023 11:58

This comment has been minimized.

Sign in to view

gsmet added the triage/needs-rebase This PR needs to be rebased first because it has merge conflicts label Feb 4, 2023

build: have vale lint only run on affected files

670bf99

gsmet force-pushed the slimmervale branch from 1bc4ff1 to 670bf99 Compare May 2, 2023 16:48

michelle-purcell mentioned this pull request Jul 20, 2023

Failed container 'unable to mount a file' error when running Quarkus local build Vale linter (DVale) on RHEL #29106

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: have vale lint only run on affected files #30378

build: have vale lint only run on affected files #30378

maxandersen commented Jan 14, 2023

quarkus-bot bot commented Jan 14, 2023

maxandersen commented Jan 14, 2023

maxandersen commented Jan 14, 2023

This comment has been minimized.

famod Jan 14, 2023

michelle-purcell Jan 16, 2023 •

edited

Loading

famod Jan 16, 2023

michelle-purcell Jan 17, 2023 •

edited

Loading

maxandersen Jan 17, 2023

michelle-purcell Jan 17, 2023

maxandersen Jan 17, 2023

famod commented Jan 14, 2023

maxandersen commented Jan 15, 2023

michelle-purcell commented Jan 16, 2023

michelle-purcell Jan 16, 2023

maxandersen Jan 16, 2023

michelle-purcell Jan 17, 2023

michelle-purcell left a comment

This comment has been minimized.

quarkus-bot bot commented May 3, 2023

build: have vale lint only run on affected files #30378

Are you sure you want to change the base?

build: have vale lint only run on affected files #30378

Conversation

maxandersen commented Jan 14, 2023

quarkus-bot bot commented Jan 14, 2023

maxandersen commented Jan 14, 2023

maxandersen commented Jan 14, 2023

This comment has been minimized.

Choose a reason for hiding this comment

michelle-purcell Jan 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michelle-purcell Jan 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

famod commented Jan 14, 2023

maxandersen commented Jan 15, 2023

michelle-purcell commented Jan 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michelle-purcell left a comment

Choose a reason for hiding this comment

This comment has been minimized.

quarkus-bot bot commented May 3, 2023

Failing Jobs - Building 670bf99

Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/jdbc/jdbc-oracle/deployment

⚙️ JVM Tests - JDK 17 #

📦 extensions/jdbc/jdbc-oracle/deployment

⚙️ JVM Tests - JDK 19 #

📦 extensions/jdbc/jdbc-oracle/deployment

michelle-purcell Jan 16, 2023 •

edited

Loading

michelle-purcell Jan 17, 2023 •

edited

Loading

Failing Jobs - Building `670bf99`