Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Darrel Schneider
+1 thanks for working on this From: Donal Evans Sent: Thursday, May 27, 2021 10:22 AM To: dev@geode.apache.org Subject: Cleaning up the codebase - use curly braces everywhere Hi Geode dev, I've recently been looking at ways to improve code quality in small ways

Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Udo Kohlmeyer
+1, I trust the work that you’ve done @Donal. I think splitting this out by package or any other metric we could come up with, it an exercise in futility. So let’s get this merged in, set up a spotless rule to make sure we don’t introduce more and move on… --Udo From: Donal Evans Date: Frida

Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Donal Evans
Regarding doing the changes at package level, there would still be a huge amount to review for certain code owners, and some code owners would get tagged to review multiple of the PRs, all of which would consist of nearly identical changes repeated over and over. For more complex change sets, br

Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Anilkumar Gingade
+1 Instead of big merge; can this be done at package level; just a thought. -Anil. On 5/27/21, 10:51 AM, "Dale Emery" wrote: We might also use IntelliJ to enforce any guidelines that we want to enforce. You can run inspections on the command line: https://nam04.safelinks.protection.ou

Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Dale Emery
We might also use IntelliJ to enforce any guidelines that we want to enforce. You can run inspections on the command line: https://www.jetbrains.com/help/idea/command-line-code-inspector.html An advantage of using IntelliJ inspections is that we can provide an inspection profile that treats vio

Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Patrick Johnson
+1. Also really like Naba's idea of incorporating this into spotless. On May 27, 2021, at 10:46 AM, Raymond Ingles mailto:ring...@vmware.com>> wrote: +1 Sounds like a good idea. Since always using curly braces is part of the style guide, fixing this makes sense. The only possible additional st

Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Raymond Ingles
+1 Sounds like a good idea. Since always using curly braces is part of the style guide, fixing this makes sense. The only possible additional step for large-but-automatically-generated-and-specifically-targeted PRs like this is running the test suite with code coverage, making sure the updated

Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Nabarun Nag
This is great clean up idea. I think you should go for it. Regarding the check, I think we can try to incorporate it in our spotless script to fix it if we miss brackets in blocks . Regards Nabarun Get Outlook for iOS From: Donal Evans

Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Sarah Abbey
This is great! Thank you for doing this, Donal!

Re: Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Jacob Barrett
Thanks you!! I believe these targeted whole source tree changes like this are great. As long as it isn’t a mix of hand rolled and automated changes I think a reviewer can trust that the if heart of the change is correct there is no reason to review all 3000 lines changed. I think it gets harder

Cleaning up the codebase - use curly braces everywhere

2021-05-27 Thread Donal Evans
Hi Geode dev, I've recently been looking at ways to improve code quality in small ways throughout the codebase, and as a starting point, I thought it would be good to make it so that we're consistently using curly braces for control flow statements everywhere, since this is something that's spe