You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@beam.apache.org by Luke Cwik <lc...@google.com> on 2021/11/11 21:28:09 UTC

Re: Beam Google Cloud FhirIO backwards incompatible change

I would prioritize ease of use for people who want to consume the IO and to
that point Apache Beam would like to make IOs look similar to minimize
learning curves across IOs. Daniel's suggestion would allow you to move the
internal details of the classes outside and maintain backwards
compatibility which is great. It is also important to control visibility of
these internal classes so marking them package private when possible makes
a lot of sense. I would also suggest taking a look at some of the existing
IOs to ensure we have a consistent style across IOs when refactoring
FhirIO. Finally the PTransform style guide[1] and IO authors guide[2] just
to make sure that we are consistently applying this guide

1: https://beam.apache.org/contribute/ptransform-style-guide/
2: https://beam.apache.org/documentation/io/developing-io-java/

On Wed, Nov 10, 2021 at 1:03 PM Daniel Collins <dp...@google.com> wrote:

> You might be able to make it not backwards incompatible if you add the
> following to FhirIO.
>
> class Read extends FhirIORead<Read> {}
>
> Then you make FhirIORead generic:
>
> class FhirIORead<ReadT extends FhirIORead<ReadT>> {...}
>
> And any method which would return FhirIORead would instead return a ReadT,
> by doing `return (ReadT)this;`.
>
> To be clear however, I personally have no objection to this change. Just
> pointing out that you don't have to make it breaking if you really don't
> want to.
>
> -Daniel
>
> On Wed, Nov 10, 2021 at 3:32 PM Milena Bukal <ms...@google.com> wrote:
>
>> Hi everyone,
>>
>> I'm looking to make a refactor in
>> https://github.com/apache/beam/pull/15925 to move the transforms in the
>> large super class FhirIO into smaller subclasses.
>>
>> This package was added for our use case and is still marked experimental.
>> Are there any objections to me making this backwards incompatible change?
>>
>> Thanks,
>> Milena
>>
>