Hi everyone! I have a few points in response.
> It is assumed that if the PR passes the standard test suite, it adheres to the code style; if not, it is a test suite problem. Such an approach makes sense in theory, but until we have enough rules, enforcing them through review is a viable option. > Enforce the *Async suffix in methods returning CompletableFuture as described in the code style. Codestyle must be fixed, if it really says it the way you did. This applies to public API only, we must state that explicitly. Some internal methods must not have *Async suffix even if they return CompletableFuture, it is a case by case decision. чт, 13 мар. 2025 г. в 09:41, Mirza Aliev <alievmi...@gmail.com>: > Hello Stan! > > Thank you for your idea! > > Link in the description leads to a non-existing page, is it intentional? > What about this already existing [1] page then? > > [1] > https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide > > чт, 13 мар. 2025 г. в 08:45, Stanislav Lukyanov <stanlukya...@gmail.com>: > > > Hello Igniters, > > > > I'd like to propose some improvements related to code style and its > > enforcement. > > > > The goal is to have sensible and easily enforceable rules added to the > > official code style guide and static checks. This will reduce the need > for > > debate and iterations during code reviews. To achieve this, it is also > > proposed to define a process for code style enforcement and evolution. > > > > # Process Additions > > > > Add the following to the [Java Code Style Guide]( > > > https://cwiki.apache.org/coInfluence/display/IGNITE/Java+Code+Style+Guide > > ). > > > > - Static enforcement > > - All code style rules should be added to the project build as static > > checks in one of the plugins, such as Checkstyle or PMD. > > - The reverse is also true -- all static analysis checks should be > > reflected in the code style. > > - To simplify development, the same rules should also be reflected, when > > possible, in the IDE code style configurations stored in the repository. > > - It's acceptable to suppress inspections when it makes sense (e.g., > with > > @SuppressWarnings). Such suppressions are approved as part of the code > > review. > > - When adding a new static check, it is expected that there may be a > > large number of existing violations. While they should ideally be > resolved, > > it is acceptable to suppress them. It's better to add a check with many > > suppressions than not to add a check at all. > > > > - PR submission and review > > - PR reviewers are strongly encouraged to create tickets for static > > analysis improvements when the PR has code style violations. > > - It is assumed that if the PR passes the standard test suite, it > > adheres to the code style; if not, it is a test suite problem. > > > > - Making changes to the code style > > - Any changes to the code style must be discussed on the dev list and > > approved with lazy consensus. > > - Since all static checks must be a part of the code style, adding a new > > static check must be discussed on the dev list (unless it is adding a > > missing check for an existing rule). > > > > # New Rules and Checks > > > > Following the proposed process, below are proposals for adding new rules. > > > > - **New rule, existing check:** There must be a newline symbol at the end > > of the file. This is already enforced but not in the code style. > > > > - **New rule and check:** Prohibit nullable collections. > > It's a recommended best practice in Java to use empty collections > instead > > of nulls. > > Currently, some of the Ignite 3 code base uses empty collections while > > some use nulls, which creates ambiguity. > > An easy way to enforce that components do not pass null for collections > > is to prohibit @Nullable on collection types. > > > > It is also proposed to add new checks for existing rules. This is > included > > mostly for illustration purposes—the proposed process does not require a > > dev list discussion for enforcing an existing rule with a static check. > > > > - **New check, existing rule:** var usage > > Enforce the var keyword usage as described in the code style. > > > > - **New check, existing rule:** Async methods > > Enforce the *Async suffix in methods returning CompletableFuture as > > described in the code style. > > > > I will be creating tickets for the new checks above shortly. > > > > Thanks, > > Stan > > > -- Sincerely yours, Ivan Bessonov
