aglinxinyuan opened a new pull request, #7709:
URL: https://github.com/apache/texera/pull/7709
### What changes were proposed in this PR?
Removes `-Xelide-below WARNING` from the sixteen `build.sbt` files that set
it, along with the comments that exist only to explain it. Build-config only,
**16 files, −20 lines**, no behaviour change.
> ⚠️ **Stacked on #7451.** The first commit here is #7451's deletion of
`ElidableStatement`; it drops out of this diff once #7451 merges. The order is
required — see *Why the order matters* below. Review the second commit,
`chore(build): remove the dead -Xelide-below scalac option`.
#### Why the flag is dead
`-Xelide-below <level>` makes scalac replace a call to an `@elidable(n)`
method with `()` when `n < level`. The only `@elidable`-annotated code in this
repo was `ElidableStatement`:
| Wrapper | Level | vs. `WARNING` (900) |
| --- | ---: | --- |
| `finest` | `FINEST` 300 | compiled out |
| `finer` | `FINER` 400 | compiled out |
| `fine` | `FINE` 500 | compiled out |
| `info` | `INFO` 800 | compiled out |
It had no callers, so nothing was actually being elided, and #7451 deletes
it. After that `git grep -in elidable` is empty and the flag switches nothing.
#### Why this is a no-op, not a silent re-enable
The obvious worry is `assert`. It is `@elidable(ASSERTION)` = **2000**,
above `WARNING`, so assertions are compiled in today — and scalac's default
with `-Xelide-below` absent elides nothing, so they are compiled in after this
too. Same for `assume`. Nothing that was elided starts running, and nothing
that was running stops.
#### Why the order matters
Dropping the flag while `ElidableStatement` still exists would break
`ElidableStatementSpec`, which asserts `counter == 0` after
`ElidableStatement.info { counter += 1 }` — true only under elision. Hence the
stack rather than a standalone PR.
#### What was removed
Thirteen modules share one line; three differ:
| File | Removed |
| --- | --- |
| `amber/build.sbt` | the whole `scalacOptions ++= Seq(...)` line + the `//
to turn on, use: INFO` / `// to turn off, use: WARNING` pair above it |
| `common/workflow-compiler/build.sbt` | two `Seq` entries (flag and value
on separate lines) |
| `computing-unit-managing-service/build.sbt` | one `Seq` entry |
| the other thirteen | one `Seq` entry each, with its `// Turn on
optimizations with "WARNING" as the threshold` trailing comment |
Full list: `access-control-service`, `amber`,
`common/{auth,config,dao,pybuilder,resource,util,workflow-compiler,workflow-core,workflow-operator}`,
`computing-unit-managing-service`, `config-service`, `file-service`,
`notebook-migration-service`, `workflow-compiling-service`.
### Any related issues, documentation, discussions?
Closes #7706
Follow-up to a review note from @Yicong-Huang on #7451.
### How was this PR tested?
Existing tests only — this is a build-config removal, and there is nothing
left to test once the one construct the flag governed is gone.
Locally, from the repo root with Java 17:
- `sbt compile Test/compile` over the root aggregate (all sixteen modules) —
success.
Verification, re-runnable by a reviewer:
```
git grep -in xelide # nothing
git grep -in elidable # nothing
```
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
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]
