[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 all commits and force push into the PR branch to update the PR. --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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. --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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, semicolons in Scala code, etc. Thanks, Fabian --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #1856: FLINK-3650 Add maxBy/minBy to Scala DataSet API
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 not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
