Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/1856
Merging
---
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 wishes so, or if the fea
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/1856
Thanks for the update @ramkrish86. PR is good 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 does not have t
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
@fhueske - Pushed with latest updates. Pls review. Thank you for guiding me
through this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/1856
Hi @ramkrish86, PR looks quite good now. I had only a few minor comments.
After those are fixed, the PR should be good to merge.
---
If your project is set up for it, you can reply to this email and
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
Force pushed as per your advice @fhueske. Ran mvn clean verify and there
are no warnings generated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/1856
I think the easiest way is to fork of a branch from the current master and
cherry-pick all your commits one after the other onto that branch (except for
the merge commit of course). Then you squash a
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
How to remove the merge commit? If I try to remove it I lose all my
commits. @fhueske - I have updated the PR. Thanks for very sharp eyes like
seeing the spaces and new lines that were missed.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/1856
Hi @ramkrish86, the PR needs another pass. Please remove also the merge
commit. Thanks, Fabian
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
@fhueske
The merge is also done now. Let me know what you thin of the recent
updates.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
Oh. Let me rebase. I did not do 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 feature
en
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
I did mvn clean verify to ensure no warning I generate. Next time will
ensure I run this command to ensure local warnings are cleared off.
---
If your project is set up for it, you can reply to t
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/1856
One more thing. It would be good to rebase the PR to the current master.
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. I
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/1856
Hi @ramkrish86, your approach is correct but the PR suffers from quite a
few Scala issues.
Please fix the problems I pointed out and double check your code for
compiler warnings, unused imports,
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
@zentol , @tillrohrmann , @fhueske
Any chance of review here. Sorry for regular reminders :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on Gi
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
This time the build seems to pass except one and there is no long lines
now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If yo
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
Thanks @zentol . I pushed a new one after wrapping the longer lines.
---
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 projec
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/1856
I did not look at it in detail, just checked whether the builds passed.
---
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
Github user ramkrish86 commented on the issue:
https://github.com/apache/flink/pull/1856
Thanks for the review @zentol . Ok. I will correct the line length. Does
the overall approach look good?
---
If your project is set up for it, you can reply to this email and have your
reply appe
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/1856
Build is failing due to 54 scala style violations. (line length)
---
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 no
19 matches
Mail list logo
