Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API to supersede components if not required #601

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nevingeorgesunny
Copy link

@nevingeorgesunny nevingeorgesunny commented Dec 10, 2024

Added a way for components to supersede other components in the support bundle creation process

@nevingeorgesunny nevingeorgesunny marked this pull request as ready for review December 17, 2024 11:55
@nevingeorgesunny nevingeorgesunny requested a review from a team as a code owner December 17, 2024 11:55
@jglick jglick changed the title adding features to supersede components if not required API to supersede components if not required Dec 17, 2024
@@ -153,6 +153,10 @@ public ComponentCategory getCategory() {
return ComponentCategory.UNCATEGORIZED;
}

public Set<Class<? extends Component>> getSupersededComponents() {
return Collections.emptySet();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or just

Suggested change
return Collections.emptySet();
return Set.of();

@@ -153,6 +153,10 @@ public ComponentCategory getCategory() {
return ComponentCategory.UNCATEGORIZED;
}

public Set<Class<? extends Component>> getSupersededComponents() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs Javadoc.

for (Component component : components) {
try {
if (supersededComponents.contains(component.getClass())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of Class in APIs is usually a bit of an antipattern since it makes assumptions about implementation hierarchies; and actually @Extension is not strictly limited to singleton classes. Maybe a boolean supersedes(Component)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants