Aggarwal-Raghav commented on PR #498: URL: https://github.com/apache/tez/pull/498#issuecomment-4474897976
> > > @abstractdog , regarding your above question on `test-patch` profile. > > > Let me share my understanding: > > > > > > 1. By default, the standard maven-compiler-plugin hides many lint warnings and Yetus relies on doing a diff of the warnings on master vs the warnings on the PR and tells that, PR has introduced 2 more warning or fixed 2 warning for instance. Frankly, I don't see much value of that because in future if JDK25 deprecates a API then this warning diff will flag it. For Tez-1.0.0 community can take a call whether to keep track of warning or remove this profile from codebase. Below table should help in making decision > > > 2. From my short experience with yetus, it compare things with the baseline master branch against your PR. I couldn't find a way to just run `mvninstall` yetus plugin or `compile` yetus plugin only on PR/patch and ignore master. > > > > > > Let me clarify a thing: > > > > > > 1. eliminating YETUS completley, is it acceptable? and move everything to standard Jenkinsfile? We'll lose the yetus summary output and spotbugs/checkstyle/linter which comes out of the box in yetus. Then it will be just like hive project. > > > > > > Feature Option 1 (Maximum Strictness) Option 2 (The Sweet Spot) Option 3 (Maximum Speed) > > > **Yetus Phases Enabled** `mvninstall`, `compile`, `unit` `compile`, `unit` > > > _(Disabled: `mvninstall`)_ `unit` only > > > _(Disabled: `mvninstall`, `compile`)_ > > > **Step 1: Baseline** `maven install: master` > > > `master compilation: pre-patch` `master compilation: pre-patch` _(Skipped entirely)_ > > > **Step 2: Patch Build** `maven install: patch` > > > `master compilation: patch` `master compilation: patch` _(Skipped entirely)_ > > > **Step 3: Test Execution** `unit: patch` `unit: patch` `unit: patch` > > > **Estimated Maven Overhead** ~20 minutes ~8 minutes 0 minutes > > > **Pros** Maximum safety. Fails early on missing dependencies. Drops JAR packaging time. Retains `test-patch` profile and early syntax failure detection and tracks compiler warning diffs. Absolute fastest pipeline. > > > **Cons** Very slow. Creates redundant JARs that `unit` doesn't strictly need. Very slight overhead to count warnings. **Loses all warning tracking** (e.g., deprecations). Syntax errors fail deep in the `unit` phase instead of failing early. > > > > I would go for "Option 1 (Maximum Strictness)", even if I like the optimization in the "Option 2 (The Sweet Spot)": > > > > 1. there could be many maven stages between `compile` and `install`, which we should cover (e.g. not sure, but maybe javadoc, etc.): using compile only could end up silently breaking things (and we're going to face them before a release) > > 2. by running all the unit tests - as that's what we're trying to achieve now - we vote for full coverage, so we tend to accept longer runtimes for maximum strictness: if it's too bad, we can optimize later, but for now, we can afford this -> given the pace of Tez project, a few longer runs are absolutely okay, so from this point of view, I personally don't care about "Creates redundant JARs that unit doesn't strictly need." > > > > regarding discontinuing Yetus: this points too far and needs community consensus: I mean that's okay, but the PR hits the local optima, I'm really satisfied with short-/mid-term > > hm, wait, I just realized again that github actions cover the `mvn clean install` scenario by running: > https://github.com/apache/tez/blob/d0fa41151f2ccdd632210689fa6bc1a184f9c0a2/.github/workflows/build.yml#L49-L50 > ``` > mvn --batch-mode --no-transfer-progress clean install -DskipTests -Dmaven.javadoc.skip=true > ``` > which is even better, as it runs on multiple platforms > > considering the minimal time difference between these options (on my laptop): > ``` > mvn --batch-mode --no-transfer-progress clean install -DskipTests -Dmaven.javadoc.skip=true > ... > [INFO] Total time: 50.257 s > ``` > > ``` > mvn --batch-mode --no-transfer-progress clean install -DskipTests > ... > [INFO] Total time: 49.779 s > ``` > > so what about running full mvn install in github actions, and going for "Option 2 (The Sweet Spot)" in yetus? But then you need to enable GitHub actions by default on the PR. I'm guessing your are triggering it manually on the PR. -- 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]
