Github user uce commented on the issue:
https://github.com/apache/flink/pull/4360
Thanks for the pointer Greg. I didn't look at the JIRA issue. I like the
idea of automating this but I would go with a pre-push hook instead of a
pre-commit hook. It could be annoying during local dev if
Github user greghogan commented on the issue:
https://github.com/apache/flink/pull/4360
@uce @StephanEwen we had a bit of continued discussion in the JIRA. Can we
add a pre-commit hook to the Apache repo to validate the format of the summary
line?
---
If your project is set up for i
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/4360
@uce I know, but this was not intentional. I squashed them but pushed the
wrong (unsquashed) branch because of my shell history.
---
If your project is set up for it, you can reply to this em
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/4360
Agree. There are two very non-descriptive commits in the master now.
Let's try to avoid that in the future...
---
If your project is set up for it, you can reply to this email and have your
r
Github user uce commented on the issue:
https://github.com/apache/flink/pull/4360
@StefanRRichter As mentioned by Greg, we should either squash follow up
commits like `Stephan's comments` into their parent or tag them similar to the
main commit with a more descriptive message.
---
I
Github user greghogan commented on the issue:
https://github.com/apache/flink/pull/4360
The merge looks to have not been squashed.
---
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 StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/4360
Merged in 219ae33d36
---
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
enabled and wish
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/4360
Thanks for the review @StephanEwen . I changed my PR according to your
comments. Will merge now.
---
If your project is set up for it, you can reply to this email and have your
reply appear o
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/4360
Looks good, +1 with request for minor change:
I would pull out the setting of the merge operator from the option
factories. We should not allow users to override (or forget to set it), be
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/4360
Version 5.5.5 is already published, so from my side this is now good to
merge. Can somebody give me another +1?
---
If your project is set up for it, you can reply to this email and have your
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/4360
Very good news, thanks for taking care of that!
---
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
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/4360
Yes, that is correct, I kindly asked them to also update their license in
the releases on their issue tracker
https://github.com/facebook/rocksdb/issues/2605.
I will update the PR as
Github user greghogan commented on the issue:
https://github.com/apache/flink/pull/4360
RocksDB 5.5.5 looks to include the desired licensing.
---
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 t
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4360
We should still be able to switch back to the original RocksDB dependency.
The licensing issue should also apply to frocksdb.
---
If your project is set up for it, you can reply to this email and hav
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/4360
Just noticed that the license is not updated in the 5.5.1 branch, but only
in master. So I guess this means we have to wait for a release that has the
updated Apache compatible license?
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/4360
CC @aljoscha - suggesting to add this change also to the 1.3.2 release.
---
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
16 matches
Mail list logo
