Build daemon ChangeProvider cleanup #3470
Conversation
|
Note that there will be some minor breakage in the internal roll, should be easy to update atomically though with the roll CL. |
|
Also open to discussing whether we should explore a different, non-breaking change here. But I don't think there are a lot of clients of this package so the breaking change is likely ok? |
| /// | ||
| /// The [collectChanges] method is a no-op for this implementation. | ||
| class AutoChangeProvider implements ChangeProvider { | ||
| class AutoChangeProviderImpl implements AutoChangeProvider { |
There was a problem hiding this comment.
Ugh, I wish I had a better suggestion than "Impl" 😢
I assume you will handle pulling this into Google3? There's only one internal client which is used by a couple tools. |
Yes, its just the one client I could see that will need an update but it is very minor |
Removes all methods from ChangeProvider, and extract only the relevant ones into separate subclasses which are now public (AutoChangeProvider vs ManualChangeProvider).
Previously these modes were conflated and the different impls provided essentially no-op implementations of the non-applicable apis.
This should fix the failures we are seeing on the bots, because the daemon was shutting down early due to an empty stream given by the manual change provider. This was broken in #3411, but we only recently published that change, and weren't using dependency overrides for the tests that were actually broken, so we only saw it after the publish.