+1 thanks for working on this ________________________________ From: Donal Evans <doev...@vmware.com> Sent: Thursday, May 27, 2021 10:22 AM To: dev@geode.apache.org <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 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 specifically called out in the Geode Code Style Guide wiki page[1] as one of the "more important points" of our code style. IntelliJ has a "Run inspection by name..." feature that makes it possible to identify all places where curly braces aren't used for control flow statements, (which showed over 3300 occurrences in the codebase) and also allows them to be automatically inserted, making the fix relatively trivial. Since this PR will touch 640 files, I wanted to make sure to first check that this is something even worth doing, and, if there's agreement that it is, to give reviewers context on what the changes are, the motivation for them, and how they were made, to help with the review process. The draft PR I have up[2] currently has no failing tests and can be marked as ready to review if there aren't any objections, and once it is, I'll try to coordinate with codeowners to get the minimal number of approvals required for a merge (it looks like only 6-7 reviewers are needed, though I'm sure that almost every code owner will be tagged as reviewers given the number of files touched). If this idea is a success, I think it would be good to have a discussion about other low-hanging code improvements we could make using static analysis (unnecessary casts, unused variables, duplicate conditions etc.), and, once a particular inspection has been "fixed," possibly consider adding a check for it as part of the PR pre-checkin to make sure it's not reintroduced. All thoughts and feedback are very welcome. Donal [1] https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FCode%2BStyle%2BGuide&data=04%7C01%7Cdarrel%40vmware.com%7C55c1f47da50548d3fa5708d92134034c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637577329548326057%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=BVvzQAdQ24ZuoOdoMrkGNJShmTkep4BGFduSAQu5H6o%3D&reserved=0 [2] https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F6523&data=04%7C01%7Cdarrel%40vmware.com%7C55c1f47da50548d3fa5708d92134034c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637577329548326057%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=gzta%2FwrhapOzp0DKcMEn1IR5XsNboWuMlUJwUwOrcGc%3D&reserved=0
