pjfanning commented on code in PR #3663:
URL: https://github.com/apache/pekko/pull/3663#discussion_r4358117588
##########
project/Paradox.scala:
##########
@@ -94,7 +97,36 @@ object Paradox {
(Compile / paradoxMarkdownToHtml / excludeFilter) := (Compile /
paradoxMarkdownToHtml / excludeFilter).value ||
ParadoxPlugin.InDirectoryFilter((Compile / paradox /
sourceDirectory).value / "includes.html"))
- lazy val groupsSettings = Seq(Compile / paradoxGroups := Map("Language" ->
Seq("Scala", "Java")))
+ lazy val groupsSettings = Seq(Compile / paradoxGroups := Map(
+ "Language" -> Seq("Scala", "Java"),
+ "BuildTool" -> Seq("sbt", "Maven", "Gradle")))
+
+ lazy val groupsValidationSettings = Seq(
+ PekkoValidatePullRequest.additionalTasks += verifyBuildToolTabs,
Review Comment:
I am not sure this verification task pays for itself. The functional change
is the one-line `paradoxGroups` addition, while this adds ~30 lines of build
code that:
- hard-codes two pages (`typed/guide/tutorial_1.html`,
`typed/cluster.html`), so moving or renaming either one breaks
`validatePullRequest`;
- extracts `<select>` blocks with a regex and parses them with
`scala.xml.XML.loadString`, which is fragile against HTML that is not
well-formed XML (e.g. a bare `selected` attribute);
- asserts on the Paradox theme's output rather than on Pekko code, so a
theme upgrade could break it for unrelated reasons.
`ParadoxWithPrValidation` already runs `Compile / paradox` during PR
validation, so a one-off manual check of the generated tabs seems sufficient.
Could we drop the task, or at least not add it to `additionalTasks`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
