You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/10/01 10:06:09 UTC

[GitHub] [flink] aljoscha commented on pull request #11326: [FLINK-11427][formats] Add protobuf parquet support for StreamingFileSink

aljoscha commented on pull request #11326:
URL: https://github.com/apache/flink/pull/11326#issuecomment-702031110


   I think we should merge this after all. The code is very simple but if we provide it we avoid different users having to re-implement it in their own code. Plus, having the code in an example as text can go out of date and I would avoid the problem that users copy the code from the documentation and it then doesn't work with the current interfaces.
   
   Also, I think we don't need to update the `NOTICE` file because the new dependencies are optional. The change doesn't introduce any new dependencies that are not optional or in `test` scope, so it's quite minimal.
   
   I do think @JingsongLi makes a good point but I think here I think it's more important that we don't have code in the documentation that can "rot". WDYT?
   
   @gaoyunhaii If we agree to merge, could you please rebase on `master` and make sure that it still works?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org