Hi @aruke, I'm sorry to have taken so long to answer, I was pretty busy at KotlinConf last week :)
Thanks for the contribution. As discussed in the issue, I'd prefer to make this change as part of the headers redesign.
There a few things to consider for this PR:
The tests for the new parameters are missing
We also need to generate and commit the ABI dumps to record the changes to the ABI (the CI will fail if we don't). This is done by running ./gradlew apiDump, and committing the resulting updates to *.api files
Now, speaking of binary compatibility, it will actually be broken if we simply modify the public constructor like this (even if there is a default value for the parameter). We should tread carefully here because binary incompatibilities cannot be worked around on the consumer side.
We would need to add new constructors instead, or generate the overloads with @JvmOverloads, but then it means one more set of constructors to stay compatible with after the redesign, hence why I'm reluctant to make this quick change here.
I promise I'll get to work on the redesign of the header classes, I hope it's ok for you to wait a little bit.
@aruke by the way, I did not forget about this - I have a bunch of changes locally, but they still haven't taken a complete form. Thanks for being patient about it 🙏
Hi @aruke, I'm sorry to have taken so long to answer, I was pretty busy at KotlinConf last week :)
Thanks for the contribution. As discussed in the issue, I'd prefer to make this change as part of the headers redesign.
There a few things to consider for this PR:
./gradlew apiDump
, and committing the resulting updates to*.api
filesNow, speaking of binary compatibility, it will actually be broken if we simply modify the public constructor like this (even if there is a default value for the parameter). We should tread carefully here because binary incompatibilities cannot be worked around on the consumer side.
We would need to add new constructors instead, or generate the overloads with
@JvmOverloads
, but then it means one more set of constructors to stay compatible with after the redesign, hence why I'm reluctant to make this quick change here.I promise I'll get to work on the redesign of the header classes, I hope it's ok for you to wait a little bit.