refactor: clean up informer configuration classes #2470
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
metacosm
commented
Jul 12, 2024
metacosm
commented
- refactor: move @InformerConfig to more appropriate package
- refactor: move InformerConfigHolder to appropriate package
- chore: remove unneeded code & dependencies
- refactor: InformerConfiguration to InformerEventSourceConfiguration
- refactor: rename inner InformerConfigurationBuilder to simply Builder
- refactor: rename InformerConfig and associated field to Informer
- refactor: rename InformerConfigHolder to InformerConfiguration
openshift-ci
bot
added
the
do-not-merge/work-in-progress
csviri
reviewed
Jul 29, 2024
|
|
||
| private ControllerConfigurationOverrider(ControllerConfiguration<R> original) { | ||
| this.finalizer = original.getFinalizerName(); | ||
| this.generationAware = original.isGenerationAware(); | ||
| this.config = InformerConfigHolder.builder(original.getResourceClass()) | ||
| this.config = InformerConfiguration.builder(original.getResourceClass()) |
There was a problem hiding this comment.
Wouldn't be it confusing that there is now an InformerEventSourceConfiguration renamed from InformerConfiguration, but there is now a new InformerConfiguration for something else?
Also an @Informer that is basically the new InformerConfiguration just represented by annotation?
Not what would be better, just might be confusing.
There was a problem hiding this comment.
We would definitely need to document it and add it to the javadoc as well so that the information is available contextually. I think this cleans things nicely this way, though, because having multiple things names InformerConfig-something was getting quite confusing, especially when some of them were not even related to informers at all…
metacosm
force-pushed
the
rename-infoconfig
branch
from
5773463
to
0b20864
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>
Fixes #2424. --------- Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
openshift-merge-robot
added
the
needs-rebase
metacosm
force-pushed
the
rename-infoconfig
branch
from
0b20864
to
be4cb47
Compare
openshift-merge-robot
removed
the
needs-rebase
csviri
approved these changes
Aug 9, 2024
csviri
added a commit
that referenced
this pull request
Aug 15, 2024
* chore: set version to 5.0.0-SNAPSHOT (#2200) Signed-off-by: Attila Mészáros <csviri@gmail.com> * improve: replace current formatting plugins with spotless plugin (#2302) Signed-off-by: Attila Mészáros <csviri@gmail.com> * fix: format after rebase Signed-off-by: Attila Mészáros <csviri@gmail.com> * bump: chore use slf4j v2 (#2406) Signed-off-by: Attila Mészáros <csviri@gmail.com> * feat: allow returning additional information from conditions (#2426) Fixes #2424. --------- Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: move @InformerConfig to more appropriate package Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: move InformerConfigHolder to appropriate package Signed-off-by: Chris Laprun <claprun@redhat.com> * chore: remove unneeded code & dependencies Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: InformerConfiguration to InformerEventSourceConfiguration Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: rename inner InformerConfigurationBuilder to simply Builder Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: rename InformerConfig and associated field to Informer Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: rename InformerConfigHolder to InformerConfiguration Signed-off-by: Chris Laprun <claprun@redhat.com> --------- Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Attila Mészáros <csviri@gmail.com>
metacosm
added a commit
that referenced
this pull request
Aug 29, 2024
* chore: set version to 5.0.0-SNAPSHOT (#2200) Signed-off-by: Attila Mészáros <csviri@gmail.com> * improve: replace current formatting plugins with spotless plugin (#2302) Signed-off-by: Attila Mészáros <csviri@gmail.com> * fix: format after rebase Signed-off-by: Attila Mészáros <csviri@gmail.com> * bump: chore use slf4j v2 (#2406) Signed-off-by: Attila Mészáros <csviri@gmail.com> * feat: allow returning additional information from conditions (#2426) Fixes #2424. --------- Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: move @InformerConfig to more appropriate package Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: move InformerConfigHolder to appropriate package Signed-off-by: Chris Laprun <claprun@redhat.com> * chore: remove unneeded code & dependencies Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: InformerConfiguration to InformerEventSourceConfiguration Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: rename inner InformerConfigurationBuilder to simply Builder Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: rename InformerConfig and associated field to Informer Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: rename InformerConfigHolder to InformerConfiguration Signed-off-by: Chris Laprun <claprun@redhat.com> --------- Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Attila Mészáros <csviri@gmail.com>
csviri
added a commit
that referenced
this pull request
Sep 20, 2024
* chore: set version to 5.0.0-SNAPSHOT (#2200) Signed-off-by: Attila Mészáros <csviri@gmail.com> * improve: replace current formatting plugins with spotless plugin (#2302) Signed-off-by: Attila Mészáros <csviri@gmail.com> * fix: format after rebase Signed-off-by: Attila Mészáros <csviri@gmail.com> * bump: chore use slf4j v2 (#2406) Signed-off-by: Attila Mészáros <csviri@gmail.com> * feat: allow returning additional information from conditions (#2426) Fixes #2424. --------- Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: move @InformerConfig to more appropriate package Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: move InformerConfigHolder to appropriate package Signed-off-by: Chris Laprun <claprun@redhat.com> * chore: remove unneeded code & dependencies Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: InformerConfiguration to InformerEventSourceConfiguration Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: rename inner InformerConfigurationBuilder to simply Builder Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: rename InformerConfig and associated field to Informer Signed-off-by: Chris Laprun <claprun@redhat.com> * refactor: rename InformerConfigHolder to InformerConfiguration Signed-off-by: Chris Laprun <claprun@redhat.com> --------- Signed-off-by: Attila Mészáros <csviri@gmail.com> Signed-off-by: Chris Laprun <claprun@redhat.com> Co-authored-by: Attila Mészáros <csviri@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.