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

[DOXIA-724] Fix order of sink wrappers in pipeline #195

Merged
merged 2 commits into from Jan 15, 2024

Conversation

kwin
Copy link
Member

@kwin kwin commented Jan 13, 2024

No description provided.

@kwin kwin requested a review from michael-o January 13, 2024 16:55
* @since 2.0.0
*/
Collection<SinkWrapperFactory> getSinkWrapperFactories();
List<SinkWrapperFactory> getSinkWrapperFactories();
Copy link
Member

Choose a reason for hiding this comment

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

Is is necessary to change the signature here?

Copy link
Member Author

@kwin kwin Jan 14, 2024

Choose a reason for hiding this comment

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

I think it was a mistake exposing that via Parser. This is clearly SPI only and therefore only belongs to AbstractParser. There it should be a protected method. List as return value makes sense for usages which try to retrieve the last element only. For me M-builds should allow to modify API.

Copy link
Member

Choose a reason for hiding this comment

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

The drop it from Parser altogether, no?

@kwin kwin force-pushed the bugfix/order-of-sink-wrappers branch from 1d9828b to 096190a Compare January 14, 2024 10:50
@kwin kwin force-pushed the bugfix/order-of-sink-wrappers branch from 096190a to 8ab4481 Compare January 14, 2024 10:59
@michael-o michael-o self-requested a review January 14, 2024 19:00
@kwin kwin merged commit a36887e into master Jan 15, 2024
38 checks passed
@kwin kwin deleted the bugfix/order-of-sink-wrappers branch January 15, 2024 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants