You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by GitBox <gi...@apache.org> on 2020/06/22 20:10:37 UTC

[GitHub] [samza] cameronlee314 opened a new pull request #1389: SAMZA-2551: Upgrade all modules to automatically use checkstyle 6.11.2 (part 1: includes samza-core)

cameronlee314 opened a new pull request #1389:
URL: https://github.com/apache/samza/pull/1389


   Issues: Samza is currently using checkstyle 6.11 which doesn't check indentation inside lambdas properly. Auto-formatting from IDEs does handle lambdas better, but then checkstyle will throw an error for the auto-formatting, so the formatting has to be updated manually.
   Changes: Upgrade samza-core to use checkstyle 6.11.2 which has improved indentation checks, and updated all of the code to match the indentation checks.
   Tests: Ran compilation and unit tests.
   API/Upgrade/Usage changes: N/A
   
   Note for reviewers: There is no expected change in functionality. There should only be whitespace changes in this PR.


----------------------------------------------------------------
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



[GitHub] [samza] cameronlee314 merged pull request #1389: SAMZA-2551: Upgrade all modules to automatically use checkstyle 6.11.2 (part 1: includes samza-core)

Posted by GitBox <gi...@apache.org>.
cameronlee314 merged pull request #1389:
URL: https://github.com/apache/samza/pull/1389


   


----------------------------------------------------------------
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



[GitHub] [samza] mynameborat commented on pull request #1389: SAMZA-2551: Upgrade all modules to automatically use checkstyle 6.11.2 (part 1: includes samza-core)

Posted by GitBox <gi...@apache.org>.
mynameborat commented on pull request #1389:
URL: https://github.com/apache/samza/pull/1389#issuecomment-647786767


   > > Do you know of a way to not lose the history with this change? Sadly `git annotate` will have this commit all over and make it harder to navigate the history.
   > > Not sure if something like this would work for us - https://medium.com/@elliotchance/reformatting-your-codebase-with-git-filter-branch-21feded7c423
   > 
   > We aren't quite losing the history. It's just that the most recent commit for the file changes. You are right that it will be harder to navigate to find the most recent change, but it is still possible to run `git blame` on older commits, so the history is still accessible.
   > Thanks for the link, but it looks like the process described in the link creates an "all new history" and modifies commit IDs (seems dangerous since it could cause issues with forks/branches), and it requires having a tool to actually run all of the formatting (and it might be a bit difficult to configure a tool to do exactly what we need).
   
   Fair enough, I was skeptical about modifying commit ids & force push too. Wanted to check if we could do anything about the navigation part. Looks good to me otherwise.


----------------------------------------------------------------
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



[GitHub] [samza] mynameborat commented on pull request #1389: SAMZA-2551: Upgrade all modules to automatically use checkstyle 6.11.2 (part 1: includes samza-core)

Posted by GitBox <gi...@apache.org>.
mynameborat commented on pull request #1389:
URL: https://github.com/apache/samza/pull/1389#issuecomment-647766632


   Do you know of a way to not lose the history with this change? Sadly `git annotate` will have this commit all over and make it harder to navigate the history.
   
   Not sure if something like this would work for us - https://medium.com/@elliotchance/reformatting-your-codebase-with-git-filter-branch-21feded7c423


----------------------------------------------------------------
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



[GitHub] [samza] cameronlee314 commented on pull request #1389: SAMZA-2551: Upgrade all modules to automatically use checkstyle 6.11.2 (part 1: includes samza-core)

Posted by GitBox <gi...@apache.org>.
cameronlee314 commented on pull request #1389:
URL: https://github.com/apache/samza/pull/1389#issuecomment-647774611


   > Do you know of a way to not lose the history with this change? Sadly `git annotate` will have this commit all over and make it harder to navigate the history.
   > 
   > Not sure if something like this would work for us - https://medium.com/@elliotchance/reformatting-your-codebase-with-git-filter-branch-21feded7c423
   
   We aren't quite losing the history. It's just that the most recent commit for the file changes. You are right that it will be harder to navigate to find the most recent change, but it is still possible to run `git blame` on older commits, so the history is still accessible.
   Thanks for the link, but it looks like the process described in the link creates an "all new history" and modifies commit IDs (seems dangerous since it could cause issues with forks/branches), and it requires having a tool to actually run all of the formatting (and it might be a bit difficult to configure a tool to do exactly what we need).


----------------------------------------------------------------
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