Copilot commented on code in PR #710:
URL: https://github.com/apache/pekko-grpc/pull/710#discussion_r3324193151
##########
sbt-plugin/src/main/scala/org/apache/pekko/grpc/sbt/PekkoGrpcPlugin.scala:
##########
@@ -122,7 +118,10 @@ object PekkoGrpcPlugin extends AutoPlugin {
configuration.value.name),
managedSourceDirectories += (pekkoGrpcCodeGeneratorSettings /
target).value,
unmanagedResourceDirectories ++= (PB.recompile /
resourceDirectories).value,
- Defaults.ConfigGlobal / watchSources ++= (PB.recompile /
sources).value,
+ Defaults.ConfigZero / watchSources := Def.uncached {
+ (Defaults.ConfigZero / watchSources).value ++
+ (PB.recompile / sources).value.map(f => WatchSource(f))
+ },
Review Comment:
The `Defaults.ConfigZero / watchSources` assignment is contributed by both
`configSettings(Compile)` and `configSettings(Test)`. Because both use `:=` on
the same key (`Defaults.ConfigZero / watchSources`), the later contribution
(Test) overrides the earlier one (Compile), so the watch sources for `Compile /
PB.recompile / sources` are lost. The previous code used `++=`, which composed
additively across configs. Please change this to an append (e.g.
`Defaults.ConfigZero / watchSources ++= (PB.recompile /
sources).value.map(WatchSource(_))`) or otherwise ensure both configs' proto
sources end up being watched.
##########
sbt-plugin/src/main/scala/org/apache/pekko/grpc/sbt/PekkoGrpcPlugin.scala:
##########
@@ -138,22 +137,29 @@ object PekkoGrpcPlugin extends AutoPlugin {
pekkoGrpcCodeGeneratorSettings.value,
pekkoGrpcGenerators.value),
PB.protoSources += sourceDirectory.value / "proto") ++
- inTask(PB.recompile)(Seq(
- includeFilter := GlobFilter("*.proto"),
- managedSourceDirectories := Nil,
- unmanagedSourceDirectories := Seq(sourceDirectory.value),
- sourceDirectories := unmanagedSourceDirectories.value ++
managedSourceDirectories.value,
- managedSources := Nil,
- unmanagedSources := {
Defaults.collectFiles(unmanagedSourceDirectories, includeFilter,
excludeFilter).value },
- sources := managedSources.value ++ unmanagedSources.value,
- managedResourceDirectories := Nil,
- unmanagedResourceDirectories := resourceDirectory.value +:
PB.protoSources.value,
- resourceDirectories := unmanagedResourceDirectories.value ++
managedResourceDirectories.value,
- managedResources := Nil,
- unmanagedResources := {
- Defaults.collectFiles(unmanagedResourceDirectories, includeFilter,
excludeFilter).value
+ Seq(
+ PB.recompile / includeFilter := GlobFilter("*.proto"),
+ PB.recompile / managedSourceDirectories := Nil,
+ PB.recompile / unmanagedSourceDirectories :=
Seq(sourceDirectory.value),
+ PB.recompile / sourceDirectories := (PB.recompile /
unmanagedSourceDirectories).value ++
+ (PB.recompile / managedSourceDirectories).value,
+ PB.recompile / managedSources := Nil,
+ PB.recompile / unmanagedSources := {
+ Defaults.collectFiles(PB.recompile / unmanagedSourceDirectories,
PB.recompile / includeFilter,
+ PB.recompile / excludeFilter).value
+ },
+ PB.recompile / sources := (PB.recompile / managedSources).value ++
(PB.recompile / unmanagedSources).value,
+ PB.recompile / managedResourceDirectories := Nil,
+ PB.recompile / unmanagedResourceDirectories := resourceDirectory.value
+: PB.protoSources.value,
+ PB.recompile / resourceDirectories := (PB.recompile /
unmanagedResourceDirectories).value ++
+ (PB.recompile / managedResourceDirectories).value,
+ PB.recompile / managedResources := Nil,
+ PB.recompile / unmanagedResources := {
+ Defaults.collectFiles(PB.recompile / unmanagedResourceDirectories,
PB.recompile / includeFilter,
+ PB.recompile / excludeFilter).value
},
- resources := managedResources.value ++ unmanagedResources.value)))
+ PB.recompile / resources := (PB.recompile / managedResources).value ++
+ (PB.recompile / unmanagedResources).value))
Review Comment:
The `PB.recompile / ...` settings (lines 140–162) are now appended outside
the `inConfig(config)(...)` block, whereas previously they were wrapped in
`inConfig(config)(... ++ inTask(PB.recompile)(...))`. Two consequences:
1. The settings are no longer scoped by `config`, so the assignments
produced by `configSettings(Compile)` and `configSettings(Test)` collide on the
same global `PB.recompile / X` keys; only the last-added config (Test) wins.
2. References like `sourceDirectory.value` and `resourceDirectory.value`
inside these blocks now resolve against the project (Zero config) rather than
against `Compile` / `Test`, so the proto source/resource directories will
differ from what the original code computed.
Please re-wrap these settings in `inConfig(config)(...)` (or
`inConfig(config)(inTask(PB.recompile)(...))` style) so that the per-config
scoping is preserved.
--
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]
