I think we might wait for Accord and transactional metadata as the last big contributions in 5.0 (if I have not forgotten something) and then we can just polish it all just before the release. There will be still some room to do the housekeeping like this after these patches lend. It is not like Accord will be in trunk on Monday and we release Tuesday ...
________________________________________ From: Maxim Muzafarov <mmu...@apache.org> Sent: Monday, July 31, 2023 23:05 To: dev@cassandra.apache.org Subject: Re: [DISCUSSION] Cassandra's code style and source code analysis NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe. Hello everyone, It's been a long time since the last discussion about the import order code style, so I want to give these changes a chance as all the major JIRA issues have already landed on the release branch so we won't affect anyone. I'd be happy to find any reviewers who are interested in helping with the next steps :-) I've updated the changes to reflect the latest checkstyle work, so here they are: https://issues.apache.org/jira/browse/CASSANDRA-17925 https://github.com/apache/cassandra/pull/2108 The changes look scary at first glance, but they're actually quite simple and in line with what we've discussed above. In short, we can divide all the affected files into two parts: the update of the code style configuration files (checkstyle + IDE configs), and the update of all the sources to match the code style. In short: - "import order" hotkey will work regardless of which IDE you are using; - updated checkstyle configuration, and IDEA, Eclipse, NetBeans configurations have been updated; - AvoidStarImport checkstyle rule applied as well; The import order we've agreed upon: java.* [blank line] javax.* [blank line] com.* [blank line] net.* [blank line] org.* [blank line] org.apache.cassandra.* [blank line] all other imports [blank line] static all other imports On Mon, 27 Feb 2023 at 13:26, Maxim Muzafarov <mmu...@apache.org> wrote: > > > I suppose it can be easy for the existing feature branches if they have a > > single commit. Don't we need to adjust each commit for multi-commit feature > > branches? > > It depends on how feature branches are maintained and developed, I > guess. My thoughts here are that the IDE's hotkeys should just work to > resolve any code-style issues that arise during rebase/maintenance. > I'm not talking about enforcing all our code-style rules but giving > developers good flexibility. The classes import order rule might be a > good example here. > > On Wed, 22 Feb 2023 at 21:27, Jacek Lewandowski > <lewandowski.ja...@gmail.com> wrote: > > > > I suppose it can be easy for the existing feature branches if they have a > > single commit. Don't we need to adjust each commit for multi-commit feature > > branches? > > > > śr., 22 lut 2023, 19:48 użytkownik Maxim Muzafarov <mmu...@apache.org> > > napisał: > >> > >> Hello everyone, > >> > >> I have created an issue CASSANDRA-18277 that may help us move forward > >> with code style changes. It only affects the way we store the IntelliJ > >> code style configuration and has no effect on any current (or any) > >> releases, so it should be safe to merge. So, once the issue is > >> resolved, every developer that checkouts a release branch will use the > >> same code style stored in that branch. This in turn makes rebasing a > >> big change like the import order [1] a really straightforward matter > >> (by pressing Crtl + Opt + O in their local branch to organize > >> imports). > >> > >> See: > >> > >> Move the IntelliJ Idea code style and inspections configuration to the > >> project's root .idea directory > >> https://issues.apache.org/jira/browse/CASSANDRA-18277 > >> > >> > >> > >> [1] https://issues.apache.org/jira/browse/CASSANDRA-17925 > >> > >> On Wed, 25 Jan 2023 at 13:05, Miklosovic, Stefan > >> <stefan.mikloso...@netapp.com> wrote: > >> > > >> > Thank you Maxim for doing this. > >> > > >> > It is nice to see this effort materialized in a PR. > >> > > >> > I would wait until bigger chunks of work are committed to trunk (like > >> > CEP-15) to not collide too much. I would say we can postpone doing this > >> > until the actual 5.0 release, last weeks before it so we would not clash > >> > with any work people would like to include in 5.0. This can go in > >> > anytime, basically. > >> > > >> > Are people on the same page? > >> > > >> > Regards > >> > > >> > ________________________________________ > >> > From: Maxim Muzafarov <mmu...@apache.org> > >> > Sent: Monday, January 23, 2023 19:46 > >> > To: dev@cassandra.apache.org > >> > Subject: Re: [DISCUSSION] Cassandra's code style and source code analysis > >> > > >> > NetApp Security WARNING: This is an external email. Do not click links > >> > or open attachments unless you recognize the sender and know the content > >> > is safe. > >> > > >> > > >> > > >> > > >> > Hello everyone, > >> > > >> > You can find the changes here: > >> > https://issues.apache.org/jira/browse/CASSANDRA-17925 > >> > > >> > While preparing the code style configuration for the Eclipse IDE, I > >> > discovered that there was no easy way to have complex grouping options > >> > for the set of packages. So we need to add extra blank lines between > >> > each group of packages so that all the configurations for Eclipse, > >> > NetBeans, IntelliJ IDEA and checkstyle are aligned. I should have > >> > checked this earlier for sure, but I only did it for static imports > >> > and some groups, my bad. The resultant configuration looks like this: > >> > > >> > java.* > >> > [blank line] > >> > javax.* > >> > [blank line] > >> > com.* > >> > [blank line] > >> > net.* > >> > [blank line] > >> > org.* > >> > [blank line] > >> > org.apache.cassandra.* > >> > [blank line] > >> > all other imports > >> > [blank line] > >> > static all other imports > >> > > >> > The pull request is here: > >> > https://github.com/apache/cassandra/pull/2108 > >> > > >> > The configuration-related changes are placed in a dedicated commit, so > >> > it should be easy to make a review: > >> > https://github.com/apache/cassandra/pull/2108/commits/84e292ddc9671a0be76ceb9304b2b9a051c2d52a > >> > > >> > ________________________________ > >> > > >> > Another important thing to mention is that the total amount of changes > >> > for organising imports is really big (more than 2000 files!), so we > >> > need to decide the right time to merge this PR. Although rebasing or > >> > merging changes to development branches should become much easier > >> > ("Accept local" + "Organize imports"), we still need to pay extra > >> > attention here to minimise the impact on major patches for the next > >> > release. > >> > > >> > On Mon, 16 Jan 2023 at 13:16, Maxim Muzafarov <mmu...@apache.org> wrote: > >> > > > >> > > Stefan, > >> > > > >> > > Thank you for bringing this topic up. I'll prepare the PR shortly with > >> > > option 4, so everyone can take a look at the amount of changes. This > >> > > does not force us to go exactly this path, but it may shed light on > >> > > changes in general. > >> > > > >> > > What exactly we're planning to do in the PR: > >> > > > >> > > 1. Checkstyle AvoidStarImport rule, so no star imports will be allowed. > >> > > 2. Checkstyle ImportOrder rule, for controlling the order. > >> > > 3. The IDE code style configuration for Intellij IDEA, NetBeans, and > >> > > Eclipse (it doesn't exist for Eclipse yet). > >> > > 4. The import order according to option 4: > >> > > > >> > > ``` > >> > > java.* > >> > > javax.* > >> > > [blank line] > >> > > com.* > >> > > net.* > >> > > org.* > >> > > [blank line] > >> > > org.apache.cassandra.* > >> > > [blank line] > >> > > all other imports > >> > > [blank line] > >> > > static all other imports > >> > > ``` > >> > > > >> > > > >> > > > >> > > On Mon, 16 Jan 2023 at 12:39, Miklosovic, Stefan > >> > > <stefan.mikloso...@netapp.com> wrote: > >> > > > > >> > > > Based on the voting we should go with option 4? > >> > > > > >> > > > Two weeks passed without anybody joining so I guess folks are all > >> > > > happy with that or this just went unnoticed? > >> > > > > >> > > > Let's give it time until the end of this week (Friday 12:00 UTC). > >> > > > > >> > > > Regards > >> > > > > >> > > > ________________________________________ > >> > > > From: Maxim Muzafarov <mmu...@apache.org> > >> > > > Sent: Tuesday, January 3, 2023 14:31 > >> > > > To: dev@cassandra.apache.org > >> > > > Subject: Re: [DISCUSSION] Cassandra's code style and source code > >> > > > analysis > >> > > > > >> > > > NetApp Security WARNING: This is an external email. Do not click > >> > > > links or open attachments unless you recognize the sender and know > >> > > > the content is safe. > >> > > > > >> > > > > >> > > > > >> > > > > >> > > > Folks, > >> > > > > >> > > > Let me update the voting status and put together everything we have > >> > > > so > >> > > > far. We definitely need more votes to have a solid foundation for > >> > > > this > >> > > > change, so I encourage everyone to consider the options above and > >> > > > share them in this thread. > >> > > > > >> > > > > >> > > > Total for each applicable option: > >> > > > > >> > > > 4-th option -- 4 votes > >> > > > 3-rd option -- 3 votes > >> > > > 5-th option -- 1 vote > >> > > > 1-st option -- 0 votes > >> > > > 2-nd option -- 0 votes > >> > > > > >> > > > On Thu, 22 Dec 2022 at 22:06, Mick Semb Wever <m...@apache.org> > >> > > > wrote: > >> > > > >> > >> > > > >> > >> > > > >> 3. Total 5 groups, 2968 files to change > >> > > > >> > >> > > > >> ``` > >> > > > >> org.apache.cassandra.* > >> > > > >> [blank line] > >> > > > >> java.* > >> > > > >> [blank line] > >> > > > >> javax.* > >> > > > >> [blank line] > >> > > > >> all other imports > >> > > > >> [blank line] > >> > > > >> static all other imports > >> > > > >> ``` > >> > > > > > >> > > > > > >> > > > > > >> > > > > 3, then 5. > >> > > > > There's lots under com.*, net.*, org.* that is essentially the > >> > > > > same as "all other imports", what's the reason to separate those? > >> > > > > > >> > > > > My preference for 3 is simply that imports are by default > >> > > > > collapsed, and if I expand them it's the dependencies on other > >> > > > > cassandra stuff I'm first grokking. It's also our only imports > >> > > > > that lead to cyclic dependencies (which we're not good at).
