Skip to content

Conversation

@liunaijie
Copy link
Member

Purpose of this pull request

subtask of #8576

Does this PR introduce any user-facing change?

How was this patch tested?

Check list


private final Map<String, Object> sinkOptionProps = new HashMap<>();

public static final Option<String> HOST =
Copy link
Member Author

Choose a reason for hiding this comment

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

Those parameters are defined, but not used. So I delete them.

@Hisoka-X
Copy link
Member

I think we should add a test case to verify the all connector has xxxSinkOptions or xxxSourceOptions.

@liunaijie
Copy link
Member Author

I think we should add a test case to verify the all connector has xxxSinkOptions or xxxSourceOptions.

Good point, I will write the test.

activeMQClient.write(serializationSchema.serialize(element));
}

@Override
Copy link
Contributor

@akulabs8 akulabs8 Jan 27, 2025

Choose a reason for hiding this comment

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

why did you remove this entire bit of code for optional?

Copy link
Member Author

Choose a reason for hiding this comment

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

The overwrite is same with parent implement, so I delete it.

@Hisoka-X
Copy link
Member

Hisoka-X commented Feb 6, 2025

Please rebase from dev.

@liunaijie liunaijie force-pushed the improve/activemq_option branch from a79618a to 52caad1 Compare February 6, 2025 03:48
@hailin0 hailin0 merged commit 629f85b into apache:dev Feb 7, 2025
5 checks passed
@liunaijie liunaijie deleted the improve/activemq_option branch February 7, 2025 06:11
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.

4 participants