Github user uce commented on the issue:
https://github.com/apache/flink/pull/2680
Thank you! I will check it out later today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user attachmentgenie commented on the issue:
https://github.com/apache/flink/pull/2680
@uce i made the changes are requested. With a few slight modifications
however.
1.
`.defaultValue(null);` throws an error so i opted to use
`.noDefaultValue();` which behaves
Github user uce commented on the issue:
https://github.com/apache/flink/pull/2680
OK, cool. If you have time to address this, I would go ahead and merge it
afterwards.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user attachmentgenie commented on the issue:
https://github.com/apache/flink/pull/2680
@uce i personally always prefer a explicit default as that keeps me in
control instead of the upstream implementation, but i can live with setting it
to `null`.
---
If your project is set
Github user attachmentgenie commented on the issue:
https://github.com/apache/flink/pull/2680
@StephanEwen seems reasonable thing to do, i updated the code to use the
new ConfigOption method
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2680
I added a new way to define configuration options (see below). It is a much
more maintainable way of defining configuration options.
Would be great if we used that for all new options
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/2680
Thank you for opening a PR for fixing this.
+1 to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
