costajohnt commented on PR #37458:
URL: https://github.com/apache/superset/pull/37458#issuecomment-4949021070
Thanks for updating the branch, glad the babel-extract fix was that simple.
On the name: my reasoning for "Subvalue" was that the whole point of the
change is that this label shows up for any aggregation, not just SUM, so I
wanted something aggregation-neutral. "Summary (Sum)" reads to me like it
reintroduces the same confusion, unless the intent is to make the suffix
dynamic per aggregator ("Summary (Avg)", "Summary (Max)", etc.), which would be
a bigger change than this PR. If the team prefers a different neutral term
("Rollup", "Aggregate", or just keeping it configurable), I'm happy to update
this PR to whatever you all settle on. I don't have strong feelings beyond
avoiding a term that implies summation.
Happy to be the PR that carries the final decision if you want to
consolidate the other three into this one.
On the codeant suggestion: `total` predates this PR (only the string
changed) and locals aren't annotated elsewhere in this module, so I left it as
is.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
