Update ApiSurface and test to improve accuracy #215
Conversation
|
R: @davorbonaci |
|
Failures look a bit like BEAM-208. |
|
|
||
| private boolean classIsAllowed(Class<?> clazz) { | ||
| return clazz.getName().startsWith("org.apache.beam"); | ||
| return clazz.getName().startsWith("org.apache.beam") |
There was a problem hiding this comment.
I'd suggest making this a set of either prefixes or Predicate<String>s. That will make it easier to add more entries than this. Then this can just be:
for (String prefix : ALLOWED_PREFIXES) {
if (clazz.getName().startsWith(prefix)) {
return true;
}
}
return false;
As a separate question, how do we deal with cases where we want to allow some parts of a package but not all of it. Eg., the testing sub-package may be different from main package.
There was a problem hiding this comment.
To allow only selected classes from a package, list them individually and don't include a prefix whitelist. When this list started I had in mind that there would be more of that, and less of these blanket allowances, hence I didn't make it just a list of allows packages. But since today it is, I will switch it over.
|
PTAL. I've sort of split the difference between going all the way to hamcrest style and maintaining a "find all errors" behavior vs downgrading to "find the first error". |
|
LGTM. Description on the matcher seems unused, so it could be elided, but its probably nice to make it an actually functioning matcher. |
|
Thanks! I figured it might eventually come in handy as a full matcher even though it isn't used now. It isn't much more work, and is a bit more self-documenting. Merging. |
[euphoria-hadoop] wrapping hadoopConfig in SequenceFileSink
* Feat: Adding composite and base job (#215) * Added python_release_candidate job. * Testing pr from rc_tag. * Adding GH_TOKEN env var * Set PR head as WORKING_BRANCH * Removed unused code. * Adding Run RC Validation job in CI.md doc file. * Removed Set Environment Variables step. Setting variables in env property. * Removed composite rc-validation action. * Rc validation workflow for Dataflow Taxi, Python Cross Validation and Runners (#227) * RC Validation Workflow (#228) Adding the following jobs: * sql_taxi_with_dataflow * python_cross_validation * generate_shared_pubsub * java_injector * direct_runner_leaderboard * dataflow_runner_leaderboard * direct_runner_gamestats * dataflow_runner_gamestats * remove_shared_pubsub * Adding extras jobs to CI.MD (#230) * Fixing workflow linter error (#231) * Using inputs in composite action (#235) * Using inputs in composite action Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local> * Added Verify Working Branch step. * Adding extras jobs to CI.MD (#230) Co-authored-by: Elias Segundo Antonio <eliassegundo.segundo@gmail.com> Co-authored-by: Elias Segundo <elias.segundo@luisrazo.local>
Closes apache#214. Closes apache#215. Closes apache#216. Co-authored-by: Christopher Wilcox <crwilcox@google.com>
2 participants
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.
This provides a better split between what is the "API surface" and what classes are allowed to be in it. Previously, there was aggressive pruning for performance reasons, but it actually missed some transitive dependency leakage.