[crac] RFR: Introduce per-Priority Context with different policies
Radim Vansa
duke at openjdk.org
Thu May 25 06:25:25 UTC 2023
On Wed, 24 May 2023 12:50:47 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:
>> Yes, but it does not explain much about the **reason** but "... as that may have a huge impact on users". I don't think there's much concern about backwards compatibility at this point; it's more important to have the best UX even in case that users do an error.
>>
>> If you want to conserve certain behaviour, write a test that will validate what happens when you attempt to register into global context in one of those notifications.
>
> Apparently you mean why the global context implementation has been changed, not why the test has to use an explicit implementation.
>
> Honestly I did not realize the global context properties were changed in #60, so I just want to revert that for a while. This was rather big change. And that deserved a separate line in the javadoc, which is btw the specification, not the tests (although they are definetely useful). I lean toward using BlockingContext, but want to think a bit more about that, with the spec change.
Yes, the behaviour should be specified through javadoc but that does not say anything about registration when the checkpoint is proceeded. Per your logic that behaviour is unspecified and hence the change doesn't matter. Moreover, you've explicitly asked to postpone javadoc changes into #65 which did not get review in 2 weeks.
Rather than changing implementation forth and back you could explain why certain behaviour is more useful. Now you've removed a test for certain codepath - creating resource in global context - if you think it should behave in a different way you should keep it and assert different outcome.
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/74#discussion_r1205045522
More information about the crac-dev
mailing list