Github user bdrillard commented on the issue:
https://github.com/apache/spark/pull/16648
@kiszk We could do that, definitely. Changes in Feature 1 (splitting excess
code among classes) are limited to the `CodeGeneration` class, and the few
`Generate...` classes included with CodeGeneration. The changes of Feature 2
begin in the `CodeGeneration` class, but then impact any class that calls
`addMutableState`.
I can either do two separate PRs, or I can refactor the commit history such
that there are two commits that can be viewed separately in this same PR, one
for each feature.
The only reason I'd hesitate to open two separate PRs (and so try to get
the commit history to separate the two major features as two commits), is that
code generation and state proliferation are tightly coupled: the test case that
I include,
[SPARK-18016](https://github.com/apache/spark/pull/16648/files#diff-c7f041fda7fd9aa1a5225f86bab4b1b0R69),
cannot be resolved with just splitting into nested sub-classes or by
compacting mutable state -- both features are required. Any class that would
require sub-classes would probably also require mutable state compaction (and
vice-versa).
So I'd suggest the strategy where we have a commit for Feature 1 and a
commit for Feature 2. That way in the PR we can choose to only view one commit
at a time during review, but the code is still tested against the more
representative test case. Would that be alright?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
