nssalian commented on issue #2062:
URL: https://github.com/apache/iceberg-go/issues/2062#issuecomment-5859778392
Thanks for putting this together. Excited to see go heading to v1.0.0.
No objection to the scope. A few notes from tracing the code, to sharpen the
Section 2/3 items before PRs start:
Section 3: I wouldn't say the four deprecations are equal cost. Two are
clean deletes, two aren't:
- `DataFileBuilder.DistinctValueCounts` - no non-test callers in-repo. Clean
delete.
- `io/gocloud.ParseAWSConfig` / `ParseGCSConfig` (the deprecated shims in
`io/gocloud/gocloud.go`) - only referenced by `io/gocloud/compat_test.go`.
Clean delete.
- `ToHumanStr` - it's a `Transform` interface method implemented by every
transform, and `ToHumanStrType` (used by `PartitionToPath`) delegates to it.
Removing it means inverting that delegation across every transform, so it's a
migration, not a delete. Worth its own PR.
- Naming nit on the last item: the deprecated symbols are
`WitMaxConcurrency` (the typo'd alias; `WithMaxConcurrency` is the one we keep)
and `RewriteFiles.Apply` (`ApplyResult` is its replacement), not the reverse.
Both fine to batch.
Suggest splitting Section 3 into "clean deletes (one PR at the tag)" and
"`ToHumanStr` migration (separate)".
Section 2: `Schema.FindFieldByIDRef` / `FieldsRef`: the
`internal.SchemaRef{}` arg already gates these to in-module callers (external
modules can't construct a type under `internal/`), so it reads more like a
deliberate sentinel than a leak. Worth pinning the target shape before someone
opens a PR.
Separately, the `//go:linkname getUpdatedPropsAndUpdateSummary` hack in
Glue/Hive/SQL can go now: `catalog/internal` already imports `catalog`, so the
helper moves there and returns `catalog.PropertiesUpdateSummary` directly, no
cycle. I went ahead and opened #2066.
Happy to help on the others but those should be doable.
--
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]
