Conversation
OSIP-1080 The project is split into several larger 'Domains'. Commits changing files within their domains can be verified independently.
Code Owners
|
sellmair
requested review from
JSMonk,
ShikaSD,
SvyatoslavScherbina,
Tapchicoma,
demiurg906,
goodwinnk,
kunyavskiy and
yanex
| import org.jetbrains.kotlin.tooling.core.withClosure | ||
| import kotlin.io.path.Path | ||
|
|
||
| /* |
There was a problem hiding this comment.
Copyright is duplicated in almost all files of the convention plugin.
There was a problem hiding this comment.
Oh, yes, this seems to have happened when I moved the files into a package. Thank you, done ✅
| ":tools:jdk-api-validator", | ||
| ":wasm:wasm.ir", | ||
| ":compiler:test-engine-sandbox", | ||
| ":repo:test-federation-runtime" |
There was a problem hiding this comment.
Please add a trailing comma.
| include: | ||
| - "libraries/tools/*gradle*/**" | ||
|
|
||
| Unknown: |
There was a problem hiding this comment.
Why is the "Compiler" domain is the root of the whole hierarchy, not the "Unknown"?
The current domain definition is incorrect because, for example, change in the standard library could be observed in the compiler tests, but since stdlib falls into "Unknown", "Compiler" domain would be considered unaffected.
There was a problem hiding this comment.
Right now, the Unknown domain was declared last so that the universal glob can match any file not matched previously. It seems like the stdlib falling into 'Unknown' might not be nice. Would you like us to declare it as its own Domain instead, or would you prefer to couple 'Unknown' and the larger 'Compiler Domain' for the first test runs?
There was a problem hiding this comment.
couple 'Unknown' and the larger 'Compiler Domain' for the first test runs?
I think this should be the way not just for the initial period, but always. Otherwise it would be quite easy to forget about updating the domains.yaml on adding new core modules to the project/refactoring existing ones and get incorrectly skipped tests.
There was a problem hiding this comment.
Good point, lets try it out like this 👍
|
|
||
| /** | ||
| * Files matching these 'glob' patterns will be included in this subsystem. | ||
| * - e.g., 'compiler/**' will include all files under the 'compiler' subdirectory |
There was a problem hiding this comment.
Intellij diff shows some zero-width spaces here, can you please check?
| @get:Inject | ||
| internal abstract val exec: ExecOperations | ||
|
|
||
| private var _diff: List<String>? = null |
There was a problem hiding this comment.
Why is this one not synchronized, and the one in AffectedDomain is?
| val out = ByteArrayOutputStream() | ||
| val err = ByteArrayOutputStream() | ||
| val result = exec.exec { | ||
| commandLine("git", "diff", "--name-only", "origin/master...HEAD") |
There was a problem hiding this comment.
Shouldn't it be relative to the merge-base?
| @@ -0,0 +1,2 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
What about also commiting IJ run configuration for that?
5 participants
OSIP-1080
The project is split into several larger 'Domains'. Commits changing files within their domains can be verified independently.
Note:
This MR will not engage the system, but allows all further owners of 'Domains' to experiment with it and annotate tests/test tasks. The rollout of a federal CI system will be gradual.