Coding guidelines. Useless JavaDoc comments.

[Denis Garus]

Igniters!

I think it's time to change coding guidelines in part of JavaDoc comments
[1]:
>> Every method, field or initializer public, private or protected in
top-level,
>> inner or anonymous type should have at least minimal Javadoc comments
including
>> description and description of parameters using @param, @return and
@throws Javadoc tags,
>> where applicable.

Let's look at JavaDoc comments in the IgniteKernal class:

Why?

/** */ - 15 matches.

What can you get new from these comments?

/** Periodic starvation check interval. */
private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30;

/** Long jvm pause detector. */
private LongJVMPauseDetector longJVMPauseDetector;

/** Scheduler. */
    private IgniteScheduler scheduler;

/** Stop guard. */
    private final AtomicBoolean stopGuard = new AtomicBoolean();

/**
     * @param cfg Configuration to use.
     * @param utilityCachePool Utility cache pool.
     * @param execSvc Executor service.
     * @param sysExecSvc System executor service.
     * @param stripedExecSvc Striped executor.
     * @param p2pExecSvc P2P executor service.
     * @param mgmtExecSvc Management executor service.
     * @param igfsExecSvc IGFS executor service.
     * @param dataStreamExecSvc data stream executor service.
     * @param restExecSvc Reset executor service.
     * @param affExecSvc Affinity executor service.
     * @param idxExecSvc Indexing executor service.
     * @param callbackExecSvc Callback executor service.
     * @param qryExecSvc Query executor service.
     * @param schemaExecSvc Schema executor service.
     * @param customExecSvcs Custom named executors.
     * @param errHnd Error handler to use for notification about startup
problems.
     * @param workerRegistry Worker registry.
     * @param hnd Default uncaught exception handler used by thread pools.
     * @throws IgniteCheckedException Thrown in case of any errors.
     */
    public void start(
        final IgniteConfiguration cfg,
        ExecutorService utilityCachePool,
        final ExecutorService execSvc,
        final ExecutorService svcExecSvc,
        final ExecutorService sysExecSvc,
        final StripedExecutor stripedExecSvc,
        ExecutorService p2pExecSvc,
        ExecutorService mgmtExecSvc,
        ExecutorService igfsExecSvc,
        StripedExecutor dataStreamExecSvc,
        ExecutorService restExecSvc,
        ExecutorService affExecSvc,
        @Nullable ExecutorService idxExecSvc,
        IgniteStripedThreadPoolExecutor callbackExecSvc,
        ExecutorService qryExecSvc,
        ExecutorService schemaExecSvc,
        @Nullable final Map<String, ? extends ExecutorService>
customExecSvcs,
        GridAbsClosure errHnd,
        WorkersRegistry workerRegistry,
        Thread.UncaughtExceptionHandler hnd,
        TimeBag startTimer
    )

These comments look ugly and useless, and that is the main class of core.
Why do they need us?
Let us change the coding guidelines in part of JavaDoc comments:
Every method public API should have at least minimal Javadoc comments,
including description and description of parameters using @param, @return,
and @throws Javadoc tags,
where applicable.
For internal API, JavaDoc comments should be optional. It's up to a
contributor or reviewer.

What are you think?

1.
https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments

[Nikolay Izhikov-2]

Hello, Denis.

Thanks for starting this discussion.

I think we have to write proper javadoc for all production classes, including internal.
We should fix useless javadoc you provide.
We should not accept patches without good javadocs.

As for the tests, seems, we can make javadoc optional.

What do you think?


В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:

> Igniters!
>
> I think it's time to change coding guidelines in part of JavaDoc comments
> [1]:
> > > Every method, field or initializer public, private or protected in
>
> top-level,
> > > inner or anonymous type should have at least minimal Javadoc comments
>
> including
> > > description and description of parameters using @param, @return and
>
> @throws Javadoc tags,
> > > where applicable.
>
> Let's look at JavaDoc comments in the IgniteKernal class:
>
> Why?
>
> /** */ - 15 matches.
>
> What can you get new from these comments?
>
> /** Periodic starvation check interval. */
> private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30;
>
> /** Long jvm pause detector. */
> private LongJVMPauseDetector longJVMPauseDetector;
>
> /** Scheduler. */
>     private IgniteScheduler scheduler;
>
> /** Stop guard. */
>     private final AtomicBoolean stopGuard = new AtomicBoolean();
>
> /**
>      * @param cfg Configuration to use.
>      * @param utilityCachePool Utility cache pool.
>      * @param execSvc Executor service.
>      * @param sysExecSvc System executor service.
>      * @param stripedExecSvc Striped executor.
>      * @param p2pExecSvc P2P executor service.
>      * @param mgmtExecSvc Management executor service.
>      * @param igfsExecSvc IGFS executor service.
>      * @param dataStreamExecSvc data stream executor service.
>      * @param restExecSvc Reset executor service.
>      * @param affExecSvc Affinity executor service.
>      * @param idxExecSvc Indexing executor service.
>      * @param callbackExecSvc Callback executor service.
>      * @param qryExecSvc Query executor service.
>      * @param schemaExecSvc Schema executor service.
>      * @param customExecSvcs Custom named executors.
>      * @param errHnd Error handler to use for notification about startup
> problems.
>      * @param workerRegistry Worker registry.
>      * @param hnd Default uncaught exception handler used by thread pools.
>      * @throws IgniteCheckedException Thrown in case of any errors.
>      */
>     public void start(
>         final IgniteConfiguration cfg,
>         ExecutorService utilityCachePool,
>         final ExecutorService execSvc,
>         final ExecutorService svcExecSvc,
>         final ExecutorService sysExecSvc,
>         final StripedExecutor stripedExecSvc,
>         ExecutorService p2pExecSvc,
>         ExecutorService mgmtExecSvc,
>         ExecutorService igfsExecSvc,
>         StripedExecutor dataStreamExecSvc,
>         ExecutorService restExecSvc,
>         ExecutorService affExecSvc,
>         @Nullable ExecutorService idxExecSvc,
>         IgniteStripedThreadPoolExecutor callbackExecSvc,
>         ExecutorService qryExecSvc,
>         ExecutorService schemaExecSvc,
>         @Nullable final Map<String, ? extends ExecutorService>
> customExecSvcs,
>         GridAbsClosure errHnd,
>         WorkersRegistry workerRegistry,
>         Thread.UncaughtExceptionHandler hnd,
>         TimeBag startTimer
>     )
>
> These comments look ugly and useless, and that is the main class of core.
> Why do they need us?
> Let us change the coding guidelines in part of JavaDoc comments:
> Every method public API should have at least minimal Javadoc comments,
> including description and description of parameters using @param, @return,
> and @throws Javadoc tags,
> where applicable.
> For internal API, JavaDoc comments should be optional. It's up to a
> contributor or reviewer.
>
> What are you think?
>
> 1.
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments

[daradurvs]

I agree that useless comments look weird in the codebase.

But, identical distance/padding/margin between fields in a class - is
really cool, and helps read the class very fast.

On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <[hidden email]> wrote:

>
> Hello, Denis.
>
> Thanks for starting this discussion.
>
> I think we have to write proper javadoc for all production classes, including internal.
> We should fix useless javadoc you provide.
> We should not accept patches without good javadocs.
>
> As for the tests, seems, we can make javadoc optional.
>
> What do you think?
>
>
> В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > Igniters!
> >
> > I think it's time to change coding guidelines in part of JavaDoc comments
> > [1]:
> > > > Every method, field or initializer public, private or protected in
> >
> > top-level,
> > > > inner or anonymous type should have at least minimal Javadoc comments
> >
> > including
> > > > description and description of parameters using @param, @return and
> >
> > @throws Javadoc tags,
> > > > where applicable.
> >
> > Let's look at JavaDoc comments in the IgniteKernal class:
> >
> > Why?
> >
> > /** */ - 15 matches.
> >
> > What can you get new from these comments?
> >
> > /** Periodic starvation check interval. */
> > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30;
> >
> > /** Long jvm pause detector. */
> > private LongJVMPauseDetector longJVMPauseDetector;
> >
> > /** Scheduler. */
> >     private IgniteScheduler scheduler;
> >
> > /** Stop guard. */
> >     private final AtomicBoolean stopGuard = new AtomicBoolean();
> >
> > /**
> >      * @param cfg Configuration to use.
> >      * @param utilityCachePool Utility cache pool.
> >      * @param execSvc Executor service.
> >      * @param sysExecSvc System executor service.
> >      * @param stripedExecSvc Striped executor.
> >      * @param p2pExecSvc P2P executor service.
> >      * @param mgmtExecSvc Management executor service.
> >      * @param igfsExecSvc IGFS executor service.
> >      * @param dataStreamExecSvc data stream executor service.
> >      * @param restExecSvc Reset executor service.
> >      * @param affExecSvc Affinity executor service.
> >      * @param idxExecSvc Indexing executor service.
> >      * @param callbackExecSvc Callback executor service.
> >      * @param qryExecSvc Query executor service.
> >      * @param schemaExecSvc Schema executor service.
> >      * @param customExecSvcs Custom named executors.
> >      * @param errHnd Error handler to use for notification about startup
> > problems.
> >      * @param workerRegistry Worker registry.
> >      * @param hnd Default uncaught exception handler used by thread pools.
> >      * @throws IgniteCheckedException Thrown in case of any errors.
> >      */
> >     public void start(
> >         final IgniteConfiguration cfg,
> >         ExecutorService utilityCachePool,
> >         final ExecutorService execSvc,
> >         final ExecutorService svcExecSvc,
> >         final ExecutorService sysExecSvc,
> >         final StripedExecutor stripedExecSvc,
> >         ExecutorService p2pExecSvc,
> >         ExecutorService mgmtExecSvc,
> >         ExecutorService igfsExecSvc,
> >         StripedExecutor dataStreamExecSvc,
> >         ExecutorService restExecSvc,
> >         ExecutorService affExecSvc,
> >         @Nullable ExecutorService idxExecSvc,
> >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> >         ExecutorService qryExecSvc,
> >         ExecutorService schemaExecSvc,
> >         @Nullable final Map<String, ? extends ExecutorService>
> > customExecSvcs,
> >         GridAbsClosure errHnd,
> >         WorkersRegistry workerRegistry,
> >         Thread.UncaughtExceptionHandler hnd,
> >         TimeBag startTimer
> >     )
> >
> > These comments look ugly and useless, and that is the main class of core.
> > Why do they need us?
> > Let us change the coding guidelines in part of JavaDoc comments:
> > Every method public API should have at least minimal Javadoc comments,
> > including description and description of parameters using @param, @return,
> > and @throws Javadoc tags,
> > where applicable.
> > For internal API, JavaDoc comments should be optional. It's up to a
> > contributor or reviewer.
> >
> > What are you think?
> >
> > 1.
> > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments

--
Best Regards, Vyacheslav D.

[Ivan Pavlukhin]

Hi,

Denis, thank you for starting this discussion!

My opinion here is that having a good javadoc for every class and
method is not feasible in the real world. I am quite curious to see a
non-trivial project which follows it. Also, all comments and javadocs
are prone to become misleading when code changes (human factor). In my
experience good method and variable names and clear code organization
often are more helpful than javadocs.

ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <[hidden email]>:

>
> I agree that useless comments look weird in the codebase.
>
> But, identical distance/padding/margin between fields in a class - is
> really cool, and helps read the class very fast.
>
> On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <[hidden email]> wrote:
> >
> > Hello, Denis.
> >
> > Thanks for starting this discussion.
> >
> > I think we have to write proper javadoc for all production classes, including internal.
> > We should fix useless javadoc you provide.
> > We should not accept patches without good javadocs.
> >
> > As for the tests, seems, we can make javadoc optional.
> >
> > What do you think?
> >
> >
> > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > Igniters!
> > >
> > > I think it's time to change coding guidelines in part of JavaDoc comments
> > > [1]:
> > > > > Every method, field or initializer public, private or protected in
> > >
> > > top-level,
> > > > > inner or anonymous type should have at least minimal Javadoc comments
> > >
> > > including
> > > > > description and description of parameters using @param, @return and
> > >
> > > @throws Javadoc tags,
> > > > > where applicable.
> > >
> > > Let's look at JavaDoc comments in the IgniteKernal class:
> > >
> > > Why?
> > >
> > > /** */ - 15 matches.
> > >
> > > What can you get new from these comments?
> > >
> > > /** Periodic starvation check interval. */
> > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30;
> > >
> > > /** Long jvm pause detector. */
> > > private LongJVMPauseDetector longJVMPauseDetector;
> > >
> > > /** Scheduler. */
> > >     private IgniteScheduler scheduler;
> > >
> > > /** Stop guard. */
> > >     private final AtomicBoolean stopGuard = new AtomicBoolean();
> > >
> > > /**
> > >      * @param cfg Configuration to use.
> > >      * @param utilityCachePool Utility cache pool.
> > >      * @param execSvc Executor service.
> > >      * @param sysExecSvc System executor service.
> > >      * @param stripedExecSvc Striped executor.
> > >      * @param p2pExecSvc P2P executor service.
> > >      * @param mgmtExecSvc Management executor service.
> > >      * @param igfsExecSvc IGFS executor service.
> > >      * @param dataStreamExecSvc data stream executor service.
> > >      * @param restExecSvc Reset executor service.
> > >      * @param affExecSvc Affinity executor service.
> > >      * @param idxExecSvc Indexing executor service.
> > >      * @param callbackExecSvc Callback executor service.
> > >      * @param qryExecSvc Query executor service.
> > >      * @param schemaExecSvc Schema executor service.
> > >      * @param customExecSvcs Custom named executors.
> > >      * @param errHnd Error handler to use for notification about startup
> > > problems.
> > >      * @param workerRegistry Worker registry.
> > >      * @param hnd Default uncaught exception handler used by thread pools.
> > >      * @throws IgniteCheckedException Thrown in case of any errors.
> > >      */
> > >     public void start(
> > >         final IgniteConfiguration cfg,
> > >         ExecutorService utilityCachePool,
> > >         final ExecutorService execSvc,
> > >         final ExecutorService svcExecSvc,
> > >         final ExecutorService sysExecSvc,
> > >         final StripedExecutor stripedExecSvc,
> > >         ExecutorService p2pExecSvc,
> > >         ExecutorService mgmtExecSvc,
> > >         ExecutorService igfsExecSvc,
> > >         StripedExecutor dataStreamExecSvc,
> > >         ExecutorService restExecSvc,
> > >         ExecutorService affExecSvc,
> > >         @Nullable ExecutorService idxExecSvc,
> > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > >         ExecutorService qryExecSvc,
> > >         ExecutorService schemaExecSvc,
> > >         @Nullable final Map<String, ? extends ExecutorService>
> > > customExecSvcs,
> > >         GridAbsClosure errHnd,
> > >         WorkersRegistry workerRegistry,
> > >         Thread.UncaughtExceptionHandler hnd,
> > >         TimeBag startTimer
> > >     )
> > >
> > > These comments look ugly and useless, and that is the main class of core.
> > > Why do they need us?
> > > Let us change the coding guidelines in part of JavaDoc comments:
> > > Every method public API should have at least minimal Javadoc comments,
> > > including description and description of parameters using @param, @return,
> > > and @throws Javadoc tags,
> > > where applicable.
> > > For internal API, JavaDoc comments should be optional. It's up to a
> > > contributor or reviewer.
> > >
> > > What are you think?
> > >
> > > 1.
> > > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
>
>
>
> --
> Best Regards, Vyacheslav D.

--
Best regards,
Ivan Pavlukhin

[Denis Garus]

Thx for feedback!

>> we have to write proper javadoc for all production classes, including
internal.

Nikolay, I cannot agree with it.

What should be the best comment for the next fields?
/** */
private static final long serialVersionUID = 0L;
or
/** */
@LoggerResource
private IgniteLogger log;

There are more than 8000 lines of /** */ only at the ignite-core module (do
not include tests).

Any comments will be redundant and just noise. Obvious comment learns
readers skip all comments.


>> identical distance/padding/margin between fields in a class - is really
cool

Vyacheslav, but we have a blank line between fields. Why is one blank line
not enough?

ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:

> Hi,
>
> Denis, thank you for starting this discussion!
>
> My opinion here is that having a good javadoc for every class and
> method is not feasible in the real world. I am quite curious to see a
> non-trivial project which follows it. Also, all comments and javadocs
> are prone to become misleading when code changes (human factor). In my
> experience good method and variable names and clear code organization
> often are more helpful than javadocs.
>
> ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <[hidden email]>:
> >
> > I agree that useless comments look weird in the codebase.
> >
> > But, identical distance/padding/margin between fields in a class - is
> > really cool, and helps read the class very fast.
> >
> > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <[hidden email]>
> wrote:
> > >
> > > Hello, Denis.
> > >
> > > Thanks for starting this discussion.
> > >
> > > I think we have to write proper javadoc for all production classes,
> including internal.
> > > We should fix useless javadoc you provide.
> > > We should not accept patches without good javadocs.
> > >
> > > As for the tests, seems, we can make javadoc optional.
> > >
> > > What do you think?
> > >
> > >
> > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > Igniters!
> > > >
> > > > I think it's time to change coding guidelines in part of JavaDoc
> comments
> > > > [1]:
> > > > > > Every method, field or initializer public, private or protected
> in
> > > >
> > > > top-level,
> > > > > > inner or anonymous type should have at least minimal Javadoc
> comments
> > > >
> > > > including
> > > > > > description and description of parameters using @param, @return
> and
> > > >
> > > > @throws Javadoc tags,
> > > > > > where applicable.
> > > >
> > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > >
> > > > Why?
> > > >
> > > > /** */ - 15 matches.
> > > >
> > > > What can you get new from these comments?
> > > >
> > > > /** Periodic starvation check interval. */
> > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 * 30;
> > > >
> > > > /** Long jvm pause detector. */
> > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > >
> > > > /** Scheduler. */
> > > >     private IgniteScheduler scheduler;
> > > >
> > > > /** Stop guard. */
> > > >     private final AtomicBoolean stopGuard = new AtomicBoolean();
> > > >
> > > > /**
> > > >      * @param cfg Configuration to use.
> > > >      * @param utilityCachePool Utility cache pool.
> > > >      * @param execSvc Executor service.
> > > >      * @param sysExecSvc System executor service.
> > > >      * @param stripedExecSvc Striped executor.
> > > >      * @param p2pExecSvc P2P executor service.
> > > >      * @param mgmtExecSvc Management executor service.
> > > >      * @param igfsExecSvc IGFS executor service.
> > > >      * @param dataStreamExecSvc data stream executor service.
> > > >      * @param restExecSvc Reset executor service.
> > > >      * @param affExecSvc Affinity executor service.
> > > >      * @param idxExecSvc Indexing executor service.
> > > >      * @param callbackExecSvc Callback executor service.
> > > >      * @param qryExecSvc Query executor service.
> > > >      * @param schemaExecSvc Schema executor service.
> > > >      * @param customExecSvcs Custom named executors.
> > > >      * @param errHnd Error handler to use for notification about
> startup
> > > > problems.
> > > >      * @param workerRegistry Worker registry.
> > > >      * @param hnd Default uncaught exception handler used by thread
> pools.
> > > >      * @throws IgniteCheckedException Thrown in case of any errors.
> > > >      */
> > > >     public void start(
> > > >         final IgniteConfiguration cfg,
> > > >         ExecutorService utilityCachePool,
> > > >         final ExecutorService execSvc,
> > > >         final ExecutorService svcExecSvc,
> > > >         final ExecutorService sysExecSvc,
> > > >         final StripedExecutor stripedExecSvc,
> > > >         ExecutorService p2pExecSvc,
> > > >         ExecutorService mgmtExecSvc,
> > > >         ExecutorService igfsExecSvc,
> > > >         StripedExecutor dataStreamExecSvc,
> > > >         ExecutorService restExecSvc,
> > > >         ExecutorService affExecSvc,
> > > >         @Nullable ExecutorService idxExecSvc,
> > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > >         ExecutorService qryExecSvc,
> > > >         ExecutorService schemaExecSvc,
> > > >         @Nullable final Map<String, ? extends ExecutorService>
> > > > customExecSvcs,
> > > >         GridAbsClosure errHnd,
> > > >         WorkersRegistry workerRegistry,
> > > >         Thread.UncaughtExceptionHandler hnd,
> > > >         TimeBag startTimer
> > > >     )
> > > >
> > > > These comments look ugly and useless, and that is the main class of
> core.
> > > > Why do they need us?
> > > > Let us change the coding guidelines in part of JavaDoc comments:
> > > > Every method public API should have at least minimal Javadoc
> comments,
> > > > including description and description of parameters using @param,
> @return,
> > > > and @throws Javadoc tags,
> > > > where applicable.
> > > > For internal API, JavaDoc comments should be optional. It's up to a
> > > > contributor or reviewer.
> > > >
> > > > What are you think?
> > > >
> > > > 1.
> > > >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> >
> >
> >
> > --
> > Best Regards, Vyacheslav D.
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>

[Andrey Kuznetsov]

+1 for making javadoc comments optional.

- Empty and tautological comments are kind of garbage that reduce
readability.
- It's better to leave the entity undocumented, than write
unexpressive/misleading comment.
- Even classes may not require javadocs, e.g. simple DTOs.

ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:

> Thx for feedback!
>
> >> we have to write proper javadoc for all production classes, including
> internal.
>
> Nikolay, I cannot agree with it.
>
> What should be the best comment for the next fields?
> /** */
> private static final long serialVersionUID = 0L;
> or
> /** */
> @LoggerResource
> private IgniteLogger log;
>
> There are more than 8000 lines of /** */ only at the ignite-core module (do
> not include tests).
>
> Any comments will be redundant and just noise. Obvious comment learns
> readers skip all comments.
>
>
> >> identical distance/padding/margin between fields in a class - is really
> cool
>
> Vyacheslav, but we have a blank line between fields. Why is one blank line
> not enough?
>
> ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:
>
> > Hi,
> >
> > Denis, thank you for starting this discussion!
> >
> > My opinion here is that having a good javadoc for every class and
> > method is not feasible in the real world. I am quite curious to see a
> > non-trivial project which follows it. Also, all comments and javadocs
> > are prone to become misleading when code changes (human factor). In my
> > experience good method and variable names and clear code organization
> > often are more helpful than javadocs.
> >
> > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <[hidden email]>:
> > >
> > > I agree that useless comments look weird in the codebase.
> > >
> > > But, identical distance/padding/margin between fields in a class - is
> > > really cool, and helps read the class very fast.
> > >
> > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <[hidden email]>
> > wrote:
> > > >
> > > > Hello, Denis.
> > > >
> > > > Thanks for starting this discussion.
> > > >
> > > > I think we have to write proper javadoc for all production classes,
> > including internal.
> > > > We should fix useless javadoc you provide.
> > > > We should not accept patches without good javadocs.
> > > >
> > > > As for the tests, seems, we can make javadoc optional.
> > > >
> > > > What do you think?
> > > >
> > > >
> > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > Igniters!
> > > > >
> > > > > I think it's time to change coding guidelines in part of JavaDoc
> > comments
> > > > > [1]:
> > > > > > > Every method, field or initializer public, private or protected
> > in
> > > > >
> > > > > top-level,
> > > > > > > inner or anonymous type should have at least minimal Javadoc
> > comments
> > > > >
> > > > > including
> > > > > > > description and description of parameters using @param, @return
> > and
> > > > >
> > > > > @throws Javadoc tags,
> > > > > > > where applicable.
> > > > >
> > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > >
> > > > > Why?
> > > > >
> > > > > /** */ - 15 matches.
> > > > >
> > > > > What can you get new from these comments?
> > > > >
> > > > > /** Periodic starvation check interval. */
> > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 *
> 30;
> > > > >
> > > > > /** Long jvm pause detector. */
> > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > >
> > > > > /** Scheduler. */
> > > > >     private IgniteScheduler scheduler;
> > > > >
> > > > > /** Stop guard. */
> > > > >     private final AtomicBoolean stopGuard = new AtomicBoolean();
> > > > >
> > > > > /**
> > > > >      * @param cfg Configuration to use.
> > > > >      * @param utilityCachePool Utility cache pool.
> > > > >      * @param execSvc Executor service.
> > > > >      * @param sysExecSvc System executor service.
> > > > >      * @param stripedExecSvc Striped executor.
> > > > >      * @param p2pExecSvc P2P executor service.
> > > > >      * @param mgmtExecSvc Management executor service.
> > > > >      * @param igfsExecSvc IGFS executor service.
> > > > >      * @param dataStreamExecSvc data stream executor service.
> > > > >      * @param restExecSvc Reset executor service.
> > > > >      * @param affExecSvc Affinity executor service.
> > > > >      * @param idxExecSvc Indexing executor service.
> > > > >      * @param callbackExecSvc Callback executor service.
> > > > >      * @param qryExecSvc Query executor service.
> > > > >      * @param schemaExecSvc Schema executor service.
> > > > >      * @param customExecSvcs Custom named executors.
> > > > >      * @param errHnd Error handler to use for notification about
> > startup
> > > > > problems.
> > > > >      * @param workerRegistry Worker registry.
> > > > >      * @param hnd Default uncaught exception handler used by thread
> > pools.
> > > > >      * @throws IgniteCheckedException Thrown in case of any errors.
> > > > >      */
> > > > >     public void start(
> > > > >         final IgniteConfiguration cfg,
> > > > >         ExecutorService utilityCachePool,
> > > > >         final ExecutorService execSvc,
> > > > >         final ExecutorService svcExecSvc,
> > > > >         final ExecutorService sysExecSvc,
> > > > >         final StripedExecutor stripedExecSvc,
> > > > >         ExecutorService p2pExecSvc,
> > > > >         ExecutorService mgmtExecSvc,
> > > > >         ExecutorService igfsExecSvc,
> > > > >         StripedExecutor dataStreamExecSvc,
> > > > >         ExecutorService restExecSvc,
> > > > >         ExecutorService affExecSvc,
> > > > >         @Nullable ExecutorService idxExecSvc,
> > > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > >         ExecutorService qryExecSvc,
> > > > >         ExecutorService schemaExecSvc,
> > > > >         @Nullable final Map<String, ? extends ExecutorService>
> > > > > customExecSvcs,
> > > > >         GridAbsClosure errHnd,
> > > > >         WorkersRegistry workerRegistry,
> > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > >         TimeBag startTimer
> > > > >     )
> > > > >
> > > > > These comments look ugly and useless, and that is the main class of
> > core.
> > > > > Why do they need us?
> > > > > Let us change the coding guidelines in part of JavaDoc comments:
> > > > > Every method public API should have at least minimal Javadoc
> > comments,
> > > > > including description and description of parameters using @param,
> > @return,
> > > > > and @throws Javadoc tags,
> > > > > where applicable.
> > > > > For internal API, JavaDoc comments should be optional. It's up to a
> > > > > contributor or reviewer.
> > > > >
> > > > > What are you think?
> > > > >
> > > > > 1.
> > > > >
> >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > >
> > >
> > >
> > > --
> > > Best Regards, Vyacheslav D.
> >
> >
> >
> > --
> > Best regards,
> > Ivan Pavlukhin
> >
>

[Roman Shtykh-2]

Although I agree we have some meaningless comments, making them optional can result at having no comments.Therefore I'd follow Nikolay's suggestion to have comments (perhaps with some exclusions for loggers, etc.) and make it optional for tests.

-- Roman
 

    On Wednesday, August 7, 2019, 7:54:51 p.m. GMT+9, Andrey Kuznetsov <[hidden email]> wrote:  
 
 +1 for making javadoc comments optional.

- Empty and tautological comments are kind of garbage that reduce
readability.
- It's better to leave the entity undocumented, than write
unexpressive/misleading comment.
- Even classes may not require javadocs, e.g. simple DTOs.

ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:

> Thx for feedback!
>
> >> we have to write proper javadoc for all production classes, including
> internal.
>
> Nikolay, I cannot agree with it.
>
> What should be the best comment for the next fields?
> /** */
> private static final long serialVersionUID = 0L;
> or
> /** */
> @LoggerResource
> private IgniteLogger log;
>
> There are more than 8000 lines of /** */ only at the ignite-core module (do
> not include tests).
>
> Any comments will be redundant and just noise. Obvious comment learns
> readers skip all comments.
>
>
> >> identical distance/padding/margin between fields in a class - is really
> cool
>
> Vyacheslav, but we have a blank line between fields. Why is one blank line
> not enough?
>
> ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:
>
> > Hi,
> >
> > Denis, thank you for starting this discussion!
> >
> > My opinion here is that having a good javadoc for every class and
> > method is not feasible in the real world. I am quite curious to see a
> > non-trivial project which follows it. Also, all comments and javadocs
> > are prone to become misleading when code changes (human factor). In my
> > experience good method and variable names and clear code organization
> > often are more helpful than javadocs.
> >
> > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <[hidden email]>:
> > >
> > > I agree that useless comments look weird in the codebase.
> > >
> > > But, identical distance/padding/margin between fields in a class - is
> > > really cool, and helps read the class very fast.
> > >
> > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <[hidden email]>
> > wrote:
> > > >
> > > > Hello, Denis.
> > > >
> > > > Thanks for starting this discussion.
> > > >
> > > > I think we have to write proper javadoc for all production classes,
> > including internal.
> > > > We should fix useless javadoc you provide.
> > > > We should not accept patches without good javadocs.
> > > >
> > > > As for the tests, seems, we can make javadoc optional.
> > > >
> > > > What do you think?
> > > >
> > > >
> > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > Igniters!
> > > > >
> > > > > I think it's time to change coding guidelines in part of JavaDoc
> > comments
> > > > > [1]:
> > > > > > > Every method, field or initializer public, private or protected
> > in
> > > > >
> > > > > top-level,
> > > > > > > inner or anonymous type should have at least minimal Javadoc
> > comments
> > > > >
> > > > > including
> > > > > > > description and description of parameters using @param, @return
> > and
> > > > >
> > > > > @throws Javadoc tags,
> > > > > > > where applicable.
> > > > >
> > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > >
> > > > > Why?
> > > > >
> > > > > /** */ - 15 matches.
> > > > >
> > > > > What can you get new from these comments?
> > > > >
> > > > > /** Periodic starvation check interval. */
> > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 *
> 30;
> > > > >
> > > > > /** Long jvm pause detector. */
> > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > >
> > > > > /** Scheduler. */
> > > > >    private IgniteScheduler scheduler;
> > > > >
> > > > > /** Stop guard. */
> > > > >    private final AtomicBoolean stopGuard = new AtomicBoolean();
> > > > >
> > > > > /**
> > > > >      * @param cfg Configuration to use.
> > > > >      * @param utilityCachePool Utility cache pool.
> > > > >      * @param execSvc Executor service.
> > > > >      * @param sysExecSvc System executor service.
> > > > >      * @param stripedExecSvc Striped executor.
> > > > >      * @param p2pExecSvc P2P executor service.
> > > > >      * @param mgmtExecSvc Management executor service.
> > > > >      * @param igfsExecSvc IGFS executor service.
> > > > >      * @param dataStreamExecSvc data stream executor service.
> > > > >      * @param restExecSvc Reset executor service.
> > > > >      * @param affExecSvc Affinity executor service.
> > > > >      * @param idxExecSvc Indexing executor service.
> > > > >      * @param callbackExecSvc Callback executor service.
> > > > >      * @param qryExecSvc Query executor service.
> > > > >      * @param schemaExecSvc Schema executor service.
> > > > >      * @param customExecSvcs Custom named executors.
> > > > >      * @param errHnd Error handler to use for notification about
> > startup
> > > > > problems.
> > > > >      * @param workerRegistry Worker registry.
> > > > >      * @param hnd Default uncaught exception handler used by thread
> > pools.
> > > > >      * @throws IgniteCheckedException Thrown in case of any errors.
> > > > >      */
> > > > >    public void start(
> > > > >        final IgniteConfiguration cfg,
> > > > >        ExecutorService utilityCachePool,
> > > > >        final ExecutorService execSvc,
> > > > >        final ExecutorService svcExecSvc,
> > > > >        final ExecutorService sysExecSvc,
> > > > >        final StripedExecutor stripedExecSvc,
> > > > >        ExecutorService p2pExecSvc,
> > > > >        ExecutorService mgmtExecSvc,
> > > > >        ExecutorService igfsExecSvc,
> > > > >        StripedExecutor dataStreamExecSvc,
> > > > >        ExecutorService restExecSvc,
> > > > >        ExecutorService affExecSvc,
> > > > >        @Nullable ExecutorService idxExecSvc,
> > > > >        IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > >        ExecutorService qryExecSvc,
> > > > >        ExecutorService schemaExecSvc,
> > > > >        @Nullable final Map<String, ? extends ExecutorService>
> > > > > customExecSvcs,
> > > > >        GridAbsClosure errHnd,
> > > > >        WorkersRegistry workerRegistry,
> > > > >        Thread.UncaughtExceptionHandler hnd,
> > > > >        TimeBag startTimer
> > > > >    )
> > > > >
> > > > > These comments look ugly and useless, and that is the main class of
> > core.
> > > > > Why do they need us?
> > > > > Let us change the coding guidelines in part of JavaDoc comments:
> > > > > Every method public API should have at least minimal Javadoc
> > comments,
> > > > > including description and description of parameters using @param,
> > @return,
> > > > > and @throws Javadoc tags,
> > > > > where applicable.
> > > > > For internal API, JavaDoc comments should be optional. It's up to a
> > > > > contributor or reviewer.
> > > > >
> > > > > What are you think?
> > > > >
> > > > > 1.
> > > > >
> >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > >
> > >
> > >
> > > --
> > > Best Regards, Vyacheslav D.
> >
> >
> >
> > --
> > Best regards,
> > Ivan Pavlukhin
> >
>  

[Mmuzaf]

Igniters,

I'm -1 with making Javadoc optional (except tests).

Here is my patch [1] with PR [2] which fixes all the Javadoc comments
for the IgniteKernal class mentioned above. Please, take a look. The
review is very appreciated.

On what we are trying to save by making Javadoc optional? In my humble
opinion - Javadoc comments are mostly concern users who debug the
Ignite when they have faced with some unhandled exception or related
to the community members with an average involvement (produces a few
small patches during the year) but not to the experienced one. Let's
just help them to read the source code.

[1] https://issues.apache.org/jira/browse/IGNITE-12051
[2] https://github.com/apache/ignite/pull/6760



On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <[hidden email]> wrote:

>
> +1 for making javadoc comments optional.
>
> - Empty and tautological comments are kind of garbage that reduce
> readability.
> - It's better to leave the entity undocumented, than write
> unexpressive/misleading comment.
> - Even classes may not require javadocs, e.g. simple DTOs.
>
> ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:
>
> > Thx for feedback!
> >
> > >> we have to write proper javadoc for all production classes, including
> > internal.
> >
> > Nikolay, I cannot agree with it.
> >
> > What should be the best comment for the next fields?
> > /** */
> > private static final long serialVersionUID = 0L;
> > or
> > /** */
> > @LoggerResource
> > private IgniteLogger log;
> >
> > There are more than 8000 lines of /** */ only at the ignite-core module (do
> > not include tests).
> >
> > Any comments will be redundant and just noise. Obvious comment learns
> > readers skip all comments.
> >
> >
> > >> identical distance/padding/margin between fields in a class - is really
> > cool
> >
> > Vyacheslav, but we have a blank line between fields. Why is one blank line
> > not enough?
> >
> > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:
> >
> > > Hi,
> > >
> > > Denis, thank you for starting this discussion!
> > >
> > > My opinion here is that having a good javadoc for every class and
> > > method is not feasible in the real world. I am quite curious to see a
> > > non-trivial project which follows it. Also, all comments and javadocs
> > > are prone to become misleading when code changes (human factor). In my
> > > experience good method and variable names and clear code organization
> > > often are more helpful than javadocs.
> > >
> > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <[hidden email]>:
> > > >
> > > > I agree that useless comments look weird in the codebase.
> > > >
> > > > But, identical distance/padding/margin between fields in a class - is
> > > > really cool, and helps read the class very fast.
> > > >
> > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <[hidden email]>
> > > wrote:
> > > > >
> > > > > Hello, Denis.
> > > > >
> > > > > Thanks for starting this discussion.
> > > > >
> > > > > I think we have to write proper javadoc for all production classes,
> > > including internal.
> > > > > We should fix useless javadoc you provide.
> > > > > We should not accept patches without good javadocs.
> > > > >
> > > > > As for the tests, seems, we can make javadoc optional.
> > > > >
> > > > > What do you think?
> > > > >
> > > > >
> > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > Igniters!
> > > > > >
> > > > > > I think it's time to change coding guidelines in part of JavaDoc
> > > comments
> > > > > > [1]:
> > > > > > > > Every method, field or initializer public, private or protected
> > > in
> > > > > >
> > > > > > top-level,
> > > > > > > > inner or anonymous type should have at least minimal Javadoc
> > > comments
> > > > > >
> > > > > > including
> > > > > > > > description and description of parameters using @param, @return
> > > and
> > > > > >
> > > > > > @throws Javadoc tags,
> > > > > > > > where applicable.
> > > > > >
> > > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > > >
> > > > > > Why?
> > > > > >
> > > > > > /** */ - 15 matches.
> > > > > >
> > > > > > What can you get new from these comments?
> > > > > >
> > > > > > /** Periodic starvation check interval. */
> > > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 *
> > 30;
> > > > > >
> > > > > > /** Long jvm pause detector. */
> > > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > > >
> > > > > > /** Scheduler. */
> > > > > >     private IgniteScheduler scheduler;
> > > > > >
> > > > > > /** Stop guard. */
> > > > > >     private final AtomicBoolean stopGuard = new AtomicBoolean();
> > > > > >
> > > > > > /**
> > > > > >      * @param cfg Configuration to use.
> > > > > >      * @param utilityCachePool Utility cache pool.
> > > > > >      * @param execSvc Executor service.
> > > > > >      * @param sysExecSvc System executor service.
> > > > > >      * @param stripedExecSvc Striped executor.
> > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > >      * @param mgmtExecSvc Management executor service.
> > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > >      * @param dataStreamExecSvc data stream executor service.
> > > > > >      * @param restExecSvc Reset executor service.
> > > > > >      * @param affExecSvc Affinity executor service.
> > > > > >      * @param idxExecSvc Indexing executor service.
> > > > > >      * @param callbackExecSvc Callback executor service.
> > > > > >      * @param qryExecSvc Query executor service.
> > > > > >      * @param schemaExecSvc Schema executor service.
> > > > > >      * @param customExecSvcs Custom named executors.
> > > > > >      * @param errHnd Error handler to use for notification about
> > > startup
> > > > > > problems.
> > > > > >      * @param workerRegistry Worker registry.
> > > > > >      * @param hnd Default uncaught exception handler used by thread
> > > pools.
> > > > > >      * @throws IgniteCheckedException Thrown in case of any errors.
> > > > > >      */
> > > > > >     public void start(
> > > > > >         final IgniteConfiguration cfg,
> > > > > >         ExecutorService utilityCachePool,
> > > > > >         final ExecutorService execSvc,
> > > > > >         final ExecutorService svcExecSvc,
> > > > > >         final ExecutorService sysExecSvc,
> > > > > >         final StripedExecutor stripedExecSvc,
> > > > > >         ExecutorService p2pExecSvc,
> > > > > >         ExecutorService mgmtExecSvc,
> > > > > >         ExecutorService igfsExecSvc,
> > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > >         ExecutorService restExecSvc,
> > > > > >         ExecutorService affExecSvc,
> > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > > >         ExecutorService qryExecSvc,
> > > > > >         ExecutorService schemaExecSvc,
> > > > > >         @Nullable final Map<String, ? extends ExecutorService>
> > > > > > customExecSvcs,
> > > > > >         GridAbsClosure errHnd,
> > > > > >         WorkersRegistry workerRegistry,
> > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > >         TimeBag startTimer
> > > > > >     )
> > > > > >
> > > > > > These comments look ugly and useless, and that is the main class of
> > > core.
> > > > > > Why do they need us?
> > > > > > Let us change the coding guidelines in part of JavaDoc comments:
> > > > > > Every method public API should have at least minimal Javadoc
> > > comments,
> > > > > > including description and description of parameters using @param,
> > > @return,
> > > > > > and @throws Javadoc tags,
> > > > > > where applicable.
> > > > > > For internal API, JavaDoc comments should be optional. It's up to a
> > > > > > contributor or reviewer.
> > > > > >
> > > > > > What are you think?
> > > > > >
> > > > > > 1.
> > > > > >
> > >
> > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > >
> > > >
> > > >
> > > > --
> > > > Best Regards, Vyacheslav D.
> > >
> > >
> > >
> > > --
> > > Best regards,
> > > Ivan Pavlukhin
> > >
> >

[Denis Garus]

Thank you, Maxim.

>>On what we are trying to save by making Javadoc optional?

Space and time.

I doubt that we need a verbal description of what private method make.
Why we need it if we just can read the code?

Bright examples:

/**
* @param helper Helper to attach to kernal context.
*/
private void addHelper(Object helper) {
    ctx.addHelper(helper);
}

/**
* Gets "on" or "off" string for given boolean value.
*
* @param b Boolean value to convert.
* @return Result string.
*/
private String onOff(boolean b) {
    return b ? "on" : "off";
}

You have to support both code of method and their JavaDoc description, but
it doesn't get any sake.

>>Let's just help them to read the source code.

But at the same time, public method IgniteKernal#start doesn't have any
description except enumeration of attributes with apparent notes.
Maybe to give it a verbal description needs one or two pages of the screen.
Does it make sense?

чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov <[hidden email]>:

> Igniters,
>
> I'm -1 with making Javadoc optional (except tests).
>
> Here is my patch [1] with PR [2] which fixes all the Javadoc comments
> for the IgniteKernal class mentioned above. Please, take a look. The
> review is very appreciated.
>
> On what we are trying to save by making Javadoc optional? In my humble
> opinion - Javadoc comments are mostly concern users who debug the
> Ignite when they have faced with some unhandled exception or related
> to the community members with an average involvement (produces a few
> small patches during the year) but not to the experienced one. Let's
> just help them to read the source code.
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12051
> [2] https://github.com/apache/ignite/pull/6760
>
>
>
> On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <[hidden email]> wrote:
> >
> > +1 for making javadoc comments optional.
> >
> > - Empty and tautological comments are kind of garbage that reduce
> > readability.
> > - It's better to leave the entity undocumented, than write
> > unexpressive/misleading comment.
> > - Even classes may not require javadocs, e.g. simple DTOs.
> >
> > ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:
> >
> > > Thx for feedback!
> > >
> > > >> we have to write proper javadoc for all production classes,
> including
> > > internal.
> > >
> > > Nikolay, I cannot agree with it.
> > >
> > > What should be the best comment for the next fields?
> > > /** */
> > > private static final long serialVersionUID = 0L;
> > > or
> > > /** */
> > > @LoggerResource
> > > private IgniteLogger log;
> > >
> > > There are more than 8000 lines of /** */ only at the ignite-core
> module (do
> > > not include tests).
> > >
> > > Any comments will be redundant and just noise. Obvious comment learns
> > > readers skip all comments.
> > >
> > >
> > > >> identical distance/padding/margin between fields in a class - is
> really
> > > cool
> > >
> > > Vyacheslav, but we have a blank line between fields. Why is one blank
> line
> > > not enough?
> > >
> > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:
> > >
> > > > Hi,
> > > >
> > > > Denis, thank you for starting this discussion!
> > > >
> > > > My opinion here is that having a good javadoc for every class and
> > > > method is not feasible in the real world. I am quite curious to see a
> > > > non-trivial project which follows it. Also, all comments and javadocs
> > > > are prone to become misleading when code changes (human factor). In
> my
> > > > experience good method and variable names and clear code organization
> > > > often are more helpful than javadocs.
> > > >
> > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <[hidden email]
> >:
> > > > >
> > > > > I agree that useless comments look weird in the codebase.
> > > > >
> > > > > But, identical distance/padding/margin between fields in a class -
> is
> > > > > really cool, and helps read the class very fast.
> > > > >
> > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> [hidden email]>
> > > > wrote:
> > > > > >
> > > > > > Hello, Denis.
> > > > > >
> > > > > > Thanks for starting this discussion.
> > > > > >
> > > > > > I think we have to write proper javadoc for all production
> classes,
> > > > including internal.
> > > > > > We should fix useless javadoc you provide.
> > > > > > We should not accept patches without good javadocs.
> > > > > >
> > > > > > As for the tests, seems, we can make javadoc optional.
> > > > > >
> > > > > > What do you think?
> > > > > >
> > > > > >
> > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > > Igniters!
> > > > > > >
> > > > > > > I think it's time to change coding guidelines in part of
> JavaDoc
> > > > comments
> > > > > > > [1]:
> > > > > > > > > Every method, field or initializer public, private or
> protected
> > > > in
> > > > > > >
> > > > > > > top-level,
> > > > > > > > > inner or anonymous type should have at least minimal
> Javadoc
> > > > comments
> > > > > > >
> > > > > > > including
> > > > > > > > > description and description of parameters using @param,
> @return
> > > > and
> > > > > > >
> > > > > > > @throws Javadoc tags,
> > > > > > > > > where applicable.
> > > > > > >
> > > > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > > > >
> > > > > > > Why?
> > > > > > >
> > > > > > > /** */ - 15 matches.
> > > > > > >
> > > > > > > What can you get new from these comments?
> > > > > > >
> > > > > > > /** Periodic starvation check interval. */
> > > > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ =
> 1000 *
> > > 30;
> > > > > > >
> > > > > > > /** Long jvm pause detector. */
> > > > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > > > >
> > > > > > > /** Scheduler. */
> > > > > > >     private IgniteScheduler scheduler;
> > > > > > >
> > > > > > > /** Stop guard. */
> > > > > > >     private final AtomicBoolean stopGuard = new
> AtomicBoolean();
> > > > > > >
> > > > > > > /**
> > > > > > >      * @param cfg Configuration to use.
> > > > > > >      * @param utilityCachePool Utility cache pool.
> > > > > > >      * @param execSvc Executor service.
> > > > > > >      * @param sysExecSvc System executor service.
> > > > > > >      * @param stripedExecSvc Striped executor.
> > > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > > >      * @param mgmtExecSvc Management executor service.
> > > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > > >      * @param dataStreamExecSvc data stream executor service.
> > > > > > >      * @param restExecSvc Reset executor service.
> > > > > > >      * @param affExecSvc Affinity executor service.
> > > > > > >      * @param idxExecSvc Indexing executor service.
> > > > > > >      * @param callbackExecSvc Callback executor service.
> > > > > > >      * @param qryExecSvc Query executor service.
> > > > > > >      * @param schemaExecSvc Schema executor service.
> > > > > > >      * @param customExecSvcs Custom named executors.
> > > > > > >      * @param errHnd Error handler to use for notification
> about
> > > > startup
> > > > > > > problems.
> > > > > > >      * @param workerRegistry Worker registry.
> > > > > > >      * @param hnd Default uncaught exception handler used by
> thread
> > > > pools.
> > > > > > >      * @throws IgniteCheckedException Thrown in case of any
> errors.
> > > > > > >      */
> > > > > > >     public void start(
> > > > > > >         final IgniteConfiguration cfg,
> > > > > > >         ExecutorService utilityCachePool,
> > > > > > >         final ExecutorService execSvc,
> > > > > > >         final ExecutorService svcExecSvc,
> > > > > > >         final ExecutorService sysExecSvc,
> > > > > > >         final StripedExecutor stripedExecSvc,
> > > > > > >         ExecutorService p2pExecSvc,
> > > > > > >         ExecutorService mgmtExecSvc,
> > > > > > >         ExecutorService igfsExecSvc,
> > > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > > >         ExecutorService restExecSvc,
> > > > > > >         ExecutorService affExecSvc,
> > > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > > > >         ExecutorService qryExecSvc,
> > > > > > >         ExecutorService schemaExecSvc,
> > > > > > >         @Nullable final Map<String, ? extends ExecutorService>
> > > > > > > customExecSvcs,
> > > > > > >         GridAbsClosure errHnd,
> > > > > > >         WorkersRegistry workerRegistry,
> > > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > > >         TimeBag startTimer
> > > > > > >     )
> > > > > > >
> > > > > > > These comments look ugly and useless, and that is the main
> class of
> > > > core.
> > > > > > > Why do they need us?
> > > > > > > Let us change the coding guidelines in part of JavaDoc
> comments:
> > > > > > > Every method public API should have at least minimal Javadoc
> > > > comments,
> > > > > > > including description and description of parameters using
> @param,
> > > > @return,
> > > > > > > and @throws Javadoc tags,
> > > > > > > where applicable.
> > > > > > > For internal API, JavaDoc comments should be optional. It's up
> to a
> > > > > > > contributor or reviewer.
> > > > > > >
> > > > > > > What are you think?
> > > > > > >
> > > > > > > 1.
> > > > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best Regards, Vyacheslav D.
> > > >
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > > Ivan Pavlukhin
> > > >
> > >
>

[Ivan Pavlukhin]

Maxim,

My main concern here is a human factor. Humans are usually not so good
in keeping documentation always in sync with a code. Examples from an
actual PR:
/**
* @param nodeId The remote node id.
* @param channel The channel to notify listeners with.
*/
private void onChannelOpened0(UUID nodeId, GridIoMessage initMsg,
Channel channel)

First, there is a mismatch between number of parameters in javadoc and
code. Second, e.g. nodeId name can be made self-descriptive rmtNodeId
name.

Mandatory javadocs do not imply good javadocs. Good javadocs do not
imply javadocs for every method and field. For me, mandatory and good
javadocs are like communism. Sounds quite nice in theory, but not
feasible in practice.

чт, 8 авг. 2019 г. в 16:55, Denis Garus <[hidden email]>:

>
> Thank you, Maxim.
>
> >>On what we are trying to save by making Javadoc optional?
>
> Space and time.
>
> I doubt that we need a verbal description of what private method make.
> Why we need it if we just can read the code?
>
> Bright examples:
>
> /**
> * @param helper Helper to attach to kernal context.
> */
> private void addHelper(Object helper) {
>     ctx.addHelper(helper);
> }
>
> /**
> * Gets "on" or "off" string for given boolean value.
> *
> * @param b Boolean value to convert.
> * @return Result string.
> */
> private String onOff(boolean b) {
>     return b ? "on" : "off";
> }
>
> You have to support both code of method and their JavaDoc description, but
> it doesn't get any sake.
>
> >>Let's just help them to read the source code.
>
> But at the same time, public method IgniteKernal#start doesn't have any
> description except enumeration of attributes with apparent notes.
> Maybe to give it a verbal description needs one or two pages of the screen.
> Does it make sense?
>
> чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov <[hidden email]>:
>
> > Igniters,
> >
> > I'm -1 with making Javadoc optional (except tests).
> >
> > Here is my patch [1] with PR [2] which fixes all the Javadoc comments
> > for the IgniteKernal class mentioned above. Please, take a look. The
> > review is very appreciated.
> >
> > On what we are trying to save by making Javadoc optional? In my humble
> > opinion - Javadoc comments are mostly concern users who debug the
> > Ignite when they have faced with some unhandled exception or related
> > to the community members with an average involvement (produces a few
> > small patches during the year) but not to the experienced one. Let's
> > just help them to read the source code.
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > [2] https://github.com/apache/ignite/pull/6760
> >
> >
> >
> > On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <[hidden email]> wrote:
> > >
> > > +1 for making javadoc comments optional.
> > >
> > > - Empty and tautological comments are kind of garbage that reduce
> > > readability.
> > > - It's better to leave the entity undocumented, than write
> > > unexpressive/misleading comment.
> > > - Even classes may not require javadocs, e.g. simple DTOs.
> > >
> > > ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:
> > >
> > > > Thx for feedback!
> > > >
> > > > >> we have to write proper javadoc for all production classes,
> > including
> > > > internal.
> > > >
> > > > Nikolay, I cannot agree with it.
> > > >
> > > > What should be the best comment for the next fields?
> > > > /** */
> > > > private static final long serialVersionUID = 0L;
> > > > or
> > > > /** */
> > > > @LoggerResource
> > > > private IgniteLogger log;
> > > >
> > > > There are more than 8000 lines of /** */ only at the ignite-core
> > module (do
> > > > not include tests).
> > > >
> > > > Any comments will be redundant and just noise. Obvious comment learns
> > > > readers skip all comments.
> > > >
> > > >
> > > > >> identical distance/padding/margin between fields in a class - is
> > really
> > > > cool
> > > >
> > > > Vyacheslav, but we have a blank line between fields. Why is one blank
> > line
> > > > not enough?
> > > >
> > > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:
> > > >
> > > > > Hi,
> > > > >
> > > > > Denis, thank you for starting this discussion!
> > > > >
> > > > > My opinion here is that having a good javadoc for every class and
> > > > > method is not feasible in the real world. I am quite curious to see a
> > > > > non-trivial project which follows it. Also, all comments and javadocs
> > > > > are prone to become misleading when code changes (human factor). In
> > my
> > > > > experience good method and variable names and clear code organization
> > > > > often are more helpful than javadocs.
> > > > >
> > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <[hidden email]
> > >:
> > > > > >
> > > > > > I agree that useless comments look weird in the codebase.
> > > > > >
> > > > > > But, identical distance/padding/margin between fields in a class -
> > is
> > > > > > really cool, and helps read the class very fast.
> > > > > >
> > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> > [hidden email]>
> > > > > wrote:
> > > > > > >
> > > > > > > Hello, Denis.
> > > > > > >
> > > > > > > Thanks for starting this discussion.
> > > > > > >
> > > > > > > I think we have to write proper javadoc for all production
> > classes,
> > > > > including internal.
> > > > > > > We should fix useless javadoc you provide.
> > > > > > > We should not accept patches without good javadocs.
> > > > > > >
> > > > > > > As for the tests, seems, we can make javadoc optional.
> > > > > > >
> > > > > > > What do you think?
> > > > > > >
> > > > > > >
> > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > > > Igniters!
> > > > > > > >
> > > > > > > > I think it's time to change coding guidelines in part of
> > JavaDoc
> > > > > comments
> > > > > > > > [1]:
> > > > > > > > > > Every method, field or initializer public, private or
> > protected
> > > > > in
> > > > > > > >
> > > > > > > > top-level,
> > > > > > > > > > inner or anonymous type should have at least minimal
> > Javadoc
> > > > > comments
> > > > > > > >
> > > > > > > > including
> > > > > > > > > > description and description of parameters using @param,
> > @return
> > > > > and
> > > > > > > >
> > > > > > > > @throws Javadoc tags,
> > > > > > > > > > where applicable.
> > > > > > > >
> > > > > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > > > > >
> > > > > > > > Why?
> > > > > > > >
> > > > > > > > /** */ - 15 matches.
> > > > > > > >
> > > > > > > > What can you get new from these comments?
> > > > > > > >
> > > > > > > > /** Periodic starvation check interval. */
> > > > > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ =
> > 1000 *
> > > > 30;
> > > > > > > >
> > > > > > > > /** Long jvm pause detector. */
> > > > > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > > > > >
> > > > > > > > /** Scheduler. */
> > > > > > > >     private IgniteScheduler scheduler;
> > > > > > > >
> > > > > > > > /** Stop guard. */
> > > > > > > >     private final AtomicBoolean stopGuard = new
> > AtomicBoolean();
> > > > > > > >
> > > > > > > > /**
> > > > > > > >      * @param cfg Configuration to use.
> > > > > > > >      * @param utilityCachePool Utility cache pool.
> > > > > > > >      * @param execSvc Executor service.
> > > > > > > >      * @param sysExecSvc System executor service.
> > > > > > > >      * @param stripedExecSvc Striped executor.
> > > > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > > > >      * @param mgmtExecSvc Management executor service.
> > > > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > > > >      * @param dataStreamExecSvc data stream executor service.
> > > > > > > >      * @param restExecSvc Reset executor service.
> > > > > > > >      * @param affExecSvc Affinity executor service.
> > > > > > > >      * @param idxExecSvc Indexing executor service.
> > > > > > > >      * @param callbackExecSvc Callback executor service.
> > > > > > > >      * @param qryExecSvc Query executor service.
> > > > > > > >      * @param schemaExecSvc Schema executor service.
> > > > > > > >      * @param customExecSvcs Custom named executors.
> > > > > > > >      * @param errHnd Error handler to use for notification
> > about
> > > > > startup
> > > > > > > > problems.
> > > > > > > >      * @param workerRegistry Worker registry.
> > > > > > > >      * @param hnd Default uncaught exception handler used by
> > thread
> > > > > pools.
> > > > > > > >      * @throws IgniteCheckedException Thrown in case of any
> > errors.
> > > > > > > >      */
> > > > > > > >     public void start(
> > > > > > > >         final IgniteConfiguration cfg,
> > > > > > > >         ExecutorService utilityCachePool,
> > > > > > > >         final ExecutorService execSvc,
> > > > > > > >         final ExecutorService svcExecSvc,
> > > > > > > >         final ExecutorService sysExecSvc,
> > > > > > > >         final StripedExecutor stripedExecSvc,
> > > > > > > >         ExecutorService p2pExecSvc,
> > > > > > > >         ExecutorService mgmtExecSvc,
> > > > > > > >         ExecutorService igfsExecSvc,
> > > > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > > > >         ExecutorService restExecSvc,
> > > > > > > >         ExecutorService affExecSvc,
> > > > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > > > > >         ExecutorService qryExecSvc,
> > > > > > > >         ExecutorService schemaExecSvc,
> > > > > > > >         @Nullable final Map<String, ? extends ExecutorService>
> > > > > > > > customExecSvcs,
> > > > > > > >         GridAbsClosure errHnd,
> > > > > > > >         WorkersRegistry workerRegistry,
> > > > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > > > >         TimeBag startTimer
> > > > > > > >     )
> > > > > > > >
> > > > > > > > These comments look ugly and useless, and that is the main
> > class of
> > > > > core.
> > > > > > > > Why do they need us?
> > > > > > > > Let us change the coding guidelines in part of JavaDoc
> > comments:
> > > > > > > > Every method public API should have at least minimal Javadoc
> > > > > comments,
> > > > > > > > including description and description of parameters using
> > @param,
> > > > > @return,
> > > > > > > > and @throws Javadoc tags,
> > > > > > > > where applicable.
> > > > > > > > For internal API, JavaDoc comments should be optional. It's up
> > to a
> > > > > > > > contributor or reviewer.
> > > > > > > >
> > > > > > > > What are you think?
> > > > > > > >
> > > > > > > > 1.
> > > > > > > >
> > > > >
> > > >
> > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best Regards, Vyacheslav D.
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best regards,
> > > > > Ivan Pavlukhin
> > > > >
> > > >
> >

--
Best regards,
Ivan Pavlukhin

[Mmuzaf]

Ivan,

It is not a problem to check Javadocs at the moment code syle checking
performed, but do we really need this? And the human-factor you
mentioned above is also related to the `self-descriptive` names. I
assume, that someone now is desiring to use single-letter variables
and single-letter class names to save space an time. We will always
have such an opinion race.

I'd prefer to leave the current situation with Javadoc as it is and
just to ask to apply the patch [1][2]. Can you? :-)

[1] https://issues.apache.org/jira/browse/IGNITE-12051
[2] https://github.com/apache/ignite/pull/6760

On Thu, 8 Aug 2019 at 17:24, Павлухин Иван <[hidden email]> wrote:

>
> Maxim,
>
> My main concern here is a human factor. Humans are usually not so good
> in keeping documentation always in sync with a code. Examples from an
> actual PR:
> /**
> * @param nodeId The remote node id.
> * @param channel The channel to notify listeners with.
> */
> private void onChannelOpened0(UUID nodeId, GridIoMessage initMsg,
> Channel channel)
>
> First, there is a mismatch between number of parameters in javadoc and
> code. Second, e.g. nodeId name can be made self-descriptive rmtNodeId
> name.
>
> Mandatory javadocs do not imply good javadocs. Good javadocs do not
> imply javadocs for every method and field. For me, mandatory and good
> javadocs are like communism. Sounds quite nice in theory, but not
> feasible in practice.
>
> чт, 8 авг. 2019 г. в 16:55, Denis Garus <[hidden email]>:
> >
> > Thank you, Maxim.
> >
> > >>On what we are trying to save by making Javadoc optional?
> >
> > Space and time.
> >
> > I doubt that we need a verbal description of what private method make.
> > Why we need it if we just can read the code?
> >
> > Bright examples:
> >
> > /**
> > * @param helper Helper to attach to kernal context.
> > */
> > private void addHelper(Object helper) {
> >     ctx.addHelper(helper);
> > }
> >
> > /**
> > * Gets "on" or "off" string for given boolean value.
> > *
> > * @param b Boolean value to convert.
> > * @return Result string.
> > */
> > private String onOff(boolean b) {
> >     return b ? "on" : "off";
> > }
> >
> > You have to support both code of method and their JavaDoc description, but
> > it doesn't get any sake.
> >
> > >>Let's just help them to read the source code.
> >
> > But at the same time, public method IgniteKernal#start doesn't have any
> > description except enumeration of attributes with apparent notes.
> > Maybe to give it a verbal description needs one or two pages of the screen.
> > Does it make sense?
> >
> > чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov <[hidden email]>:
> >
> > > Igniters,
> > >
> > > I'm -1 with making Javadoc optional (except tests).
> > >
> > > Here is my patch [1] with PR [2] which fixes all the Javadoc comments
> > > for the IgniteKernal class mentioned above. Please, take a look. The
> > > review is very appreciated.
> > >
> > > On what we are trying to save by making Javadoc optional? In my humble
> > > opinion - Javadoc comments are mostly concern users who debug the
> > > Ignite when they have faced with some unhandled exception or related
> > > to the community members with an average involvement (produces a few
> > > small patches during the year) but not to the experienced one. Let's
> > > just help them to read the source code.
> > >
> > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > [2] https://github.com/apache/ignite/pull/6760
> > >
> > >
> > >
> > > On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <[hidden email]> wrote:
> > > >
> > > > +1 for making javadoc comments optional.
> > > >
> > > > - Empty and tautological comments are kind of garbage that reduce
> > > > readability.
> > > > - It's better to leave the entity undocumented, than write
> > > > unexpressive/misleading comment.
> > > > - Even classes may not require javadocs, e.g. simple DTOs.
> > > >
> > > > ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:
> > > >
> > > > > Thx for feedback!
> > > > >
> > > > > >> we have to write proper javadoc for all production classes,
> > > including
> > > > > internal.
> > > > >
> > > > > Nikolay, I cannot agree with it.
> > > > >
> > > > > What should be the best comment for the next fields?
> > > > > /** */
> > > > > private static final long serialVersionUID = 0L;
> > > > > or
> > > > > /** */
> > > > > @LoggerResource
> > > > > private IgniteLogger log;
> > > > >
> > > > > There are more than 8000 lines of /** */ only at the ignite-core
> > > module (do
> > > > > not include tests).
> > > > >
> > > > > Any comments will be redundant and just noise. Obvious comment learns
> > > > > readers skip all comments.
> > > > >
> > > > >
> > > > > >> identical distance/padding/margin between fields in a class - is
> > > really
> > > > > cool
> > > > >
> > > > > Vyacheslav, but we have a blank line between fields. Why is one blank
> > > line
> > > > > not enough?
> > > > >
> > > > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Denis, thank you for starting this discussion!
> > > > > >
> > > > > > My opinion here is that having a good javadoc for every class and
> > > > > > method is not feasible in the real world. I am quite curious to see a
> > > > > > non-trivial project which follows it. Also, all comments and javadocs
> > > > > > are prone to become misleading when code changes (human factor). In
> > > my
> > > > > > experience good method and variable names and clear code organization
> > > > > > often are more helpful than javadocs.
> > > > > >
> > > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <[hidden email]
> > > >:
> > > > > > >
> > > > > > > I agree that useless comments look weird in the codebase.
> > > > > > >
> > > > > > > But, identical distance/padding/margin between fields in a class -
> > > is
> > > > > > > really cool, and helps read the class very fast.
> > > > > > >
> > > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> > > [hidden email]>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > Hello, Denis.
> > > > > > > >
> > > > > > > > Thanks for starting this discussion.
> > > > > > > >
> > > > > > > > I think we have to write proper javadoc for all production
> > > classes,
> > > > > > including internal.
> > > > > > > > We should fix useless javadoc you provide.
> > > > > > > > We should not accept patches without good javadocs.
> > > > > > > >
> > > > > > > > As for the tests, seems, we can make javadoc optional.
> > > > > > > >
> > > > > > > > What do you think?
> > > > > > > >
> > > > > > > >
> > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > > > > Igniters!
> > > > > > > > >
> > > > > > > > > I think it's time to change coding guidelines in part of
> > > JavaDoc
> > > > > > comments
> > > > > > > > > [1]:
> > > > > > > > > > > Every method, field or initializer public, private or
> > > protected
> > > > > > in
> > > > > > > > >
> > > > > > > > > top-level,
> > > > > > > > > > > inner or anonymous type should have at least minimal
> > > Javadoc
> > > > > > comments
> > > > > > > > >
> > > > > > > > > including
> > > > > > > > > > > description and description of parameters using @param,
> > > @return
> > > > > > and
> > > > > > > > >
> > > > > > > > > @throws Javadoc tags,
> > > > > > > > > > > where applicable.
> > > > > > > > >
> > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > > > > > >
> > > > > > > > > Why?
> > > > > > > > >
> > > > > > > > > /** */ - 15 matches.
> > > > > > > > >
> > > > > > > > > What can you get new from these comments?
> > > > > > > > >
> > > > > > > > > /** Periodic starvation check interval. */
> > > > > > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ =
> > > 1000 *
> > > > > 30;
> > > > > > > > >
> > > > > > > > > /** Long jvm pause detector. */
> > > > > > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > > > > > >
> > > > > > > > > /** Scheduler. */
> > > > > > > > >     private IgniteScheduler scheduler;
> > > > > > > > >
> > > > > > > > > /** Stop guard. */
> > > > > > > > >     private final AtomicBoolean stopGuard = new
> > > AtomicBoolean();
> > > > > > > > >
> > > > > > > > > /**
> > > > > > > > >      * @param cfg Configuration to use.
> > > > > > > > >      * @param utilityCachePool Utility cache pool.
> > > > > > > > >      * @param execSvc Executor service.
> > > > > > > > >      * @param sysExecSvc System executor service.
> > > > > > > > >      * @param stripedExecSvc Striped executor.
> > > > > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > > > > >      * @param mgmtExecSvc Management executor service.
> > > > > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > > > > >      * @param dataStreamExecSvc data stream executor service.
> > > > > > > > >      * @param restExecSvc Reset executor service.
> > > > > > > > >      * @param affExecSvc Affinity executor service.
> > > > > > > > >      * @param idxExecSvc Indexing executor service.
> > > > > > > > >      * @param callbackExecSvc Callback executor service.
> > > > > > > > >      * @param qryExecSvc Query executor service.
> > > > > > > > >      * @param schemaExecSvc Schema executor service.
> > > > > > > > >      * @param customExecSvcs Custom named executors.
> > > > > > > > >      * @param errHnd Error handler to use for notification
> > > about
> > > > > > startup
> > > > > > > > > problems.
> > > > > > > > >      * @param workerRegistry Worker registry.
> > > > > > > > >      * @param hnd Default uncaught exception handler used by
> > > thread
> > > > > > pools.
> > > > > > > > >      * @throws IgniteCheckedException Thrown in case of any
> > > errors.
> > > > > > > > >      */
> > > > > > > > >     public void start(
> > > > > > > > >         final IgniteConfiguration cfg,
> > > > > > > > >         ExecutorService utilityCachePool,
> > > > > > > > >         final ExecutorService execSvc,
> > > > > > > > >         final ExecutorService svcExecSvc,
> > > > > > > > >         final ExecutorService sysExecSvc,
> > > > > > > > >         final StripedExecutor stripedExecSvc,
> > > > > > > > >         ExecutorService p2pExecSvc,
> > > > > > > > >         ExecutorService mgmtExecSvc,
> > > > > > > > >         ExecutorService igfsExecSvc,
> > > > > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > > > > >         ExecutorService restExecSvc,
> > > > > > > > >         ExecutorService affExecSvc,
> > > > > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > > > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > > > > > >         ExecutorService qryExecSvc,
> > > > > > > > >         ExecutorService schemaExecSvc,
> > > > > > > > >         @Nullable final Map<String, ? extends ExecutorService>
> > > > > > > > > customExecSvcs,
> > > > > > > > >         GridAbsClosure errHnd,
> > > > > > > > >         WorkersRegistry workerRegistry,
> > > > > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > > > > >         TimeBag startTimer
> > > > > > > > >     )
> > > > > > > > >
> > > > > > > > > These comments look ugly and useless, and that is the main
> > > class of
> > > > > > core.
> > > > > > > > > Why do they need us?
> > > > > > > > > Let us change the coding guidelines in part of JavaDoc
> > > comments:
> > > > > > > > > Every method public API should have at least minimal Javadoc
> > > > > > comments,
> > > > > > > > > including description and description of parameters using
> > > @param,
> > > > > > @return,
> > > > > > > > > and @throws Javadoc tags,
> > > > > > > > > where applicable.
> > > > > > > > > For internal API, JavaDoc comments should be optional. It's up
> > > to a
> > > > > > > > > contributor or reviewer.
> > > > > > > > >
> > > > > > > > > What are you think?
> > > > > > > > >
> > > > > > > > > 1.
> > > > > > > > >
> > > > > >
> > > > >
> > > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Best Regards, Vyacheslav D.
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best regards,
> > > > > > Ivan Pavlukhin
> > > > > >
> > > > >
> > >
>
>
>
> --
> Best regards,
> Ivan Pavlukhin

[Garrett Alley]

Hello Igniters,

[A quick introduction: My name is Garrett and I manage documentation at
GridGain. I'm relatively new here — I've been at GridGain for 4 months —
but I've been in Documentation for most of my career.]

I put together some _proposed_ guidelines for Javadocs as a starting point
for us to discuss, *modify*, and hopefully eventually adopt.

These are based on what I've seen succeed at other companies/on the
internet:

https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=123900577

For me, the reason I prefer optional comments is that I don't want users to
have to wade through the obvious stuff to find the useful/insightful
information. If we can reduce the amount of comments, but still retain the
useful information, the readers will be rewarded.

Thanks!

===

Garrett Alley
Documentation
GridGain Systems


On Thu, Aug 8, 2019 at 7:52 AM Maxim Muzafarov <[hidden email]> wrote:

> Ivan,
>
> It is not a problem to check Javadocs at the moment code syle checking
> performed, but do we really need this? And the human-factor you
> mentioned above is also related to the `self-descriptive` names. I
> assume, that someone now is desiring to use single-letter variables
> and single-letter class names to save space an time. We will always
> have such an opinion race.
>
> I'd prefer to leave the current situation with Javadoc as it is and
> just to ask to apply the patch [1][2]. Can you? :-)
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12051
> [2] https://github.com/apache/ignite/pull/6760
>
> On Thu, 8 Aug 2019 at 17:24, Павлухин Иван <[hidden email]> wrote:
> >
> > Maxim,
> >
> > My main concern here is a human factor. Humans are usually not so good
> > in keeping documentation always in sync with a code. Examples from an
> > actual PR:
> > /**
> > * @param nodeId The remote node id.
> > * @param channel The channel to notify listeners with.
> > */
> > private void onChannelOpened0(UUID nodeId, GridIoMessage initMsg,
> > Channel channel)
> >
> > First, there is a mismatch between number of parameters in javadoc and
> > code. Second, e.g. nodeId name can be made self-descriptive rmtNodeId
> > name.
> >
> > Mandatory javadocs do not imply good javadocs. Good javadocs do not
> > imply javadocs for every method and field. For me, mandatory and good
> > javadocs are like communism. Sounds quite nice in theory, but not
> > feasible in practice.
> >
> > чт, 8 авг. 2019 г. в 16:55, Denis Garus <[hidden email]>:
> > >
> > > Thank you, Maxim.
> > >
> > > >>On what we are trying to save by making Javadoc optional?
> > >
> > > Space and time.
> > >
> > > I doubt that we need a verbal description of what private method make.
> > > Why we need it if we just can read the code?
> > >
> > > Bright examples:
> > >
> > > /**
> > > * @param helper Helper to attach to kernal context.
> > > */
> > > private void addHelper(Object helper) {
> > >     ctx.addHelper(helper);
> > > }
> > >
> > > /**
> > > * Gets "on" or "off" string for given boolean value.
> > > *
> > > * @param b Boolean value to convert.
> > > * @return Result string.
> > > */
> > > private String onOff(boolean b) {
> > >     return b ? "on" : "off";
> > > }
> > >
> > > You have to support both code of method and their JavaDoc description,
> but
> > > it doesn't get any sake.
> > >
> > > >>Let's just help them to read the source code.
> > >
> > > But at the same time, public method IgniteKernal#start doesn't have any
> > > description except enumeration of attributes with apparent notes.
> > > Maybe to give it a verbal description needs one or two pages of the
> screen.
> > > Does it make sense?
> > >
> > > чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov <[hidden email]>:
> > >
> > > > Igniters,
> > > >
> > > > I'm -1 with making Javadoc optional (except tests).
> > > >
> > > > Here is my patch [1] with PR [2] which fixes all the Javadoc comments
> > > > for the IgniteKernal class mentioned above. Please, take a look. The
> > > > review is very appreciated.
> > > >
> > > > On what we are trying to save by making Javadoc optional? In my
> humble
> > > > opinion - Javadoc comments are mostly concern users who debug the
> > > > Ignite when they have faced with some unhandled exception or related
> > > > to the community members with an average involvement (produces a few
> > > > small patches during the year) but not to the experienced one. Let's
> > > > just help them to read the source code.
> > > >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > > [2] https://github.com/apache/ignite/pull/6760
> > > >
> > > >
> > > >
> > > > On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <[hidden email]>
> wrote:
> > > > >
> > > > > +1 for making javadoc comments optional.
> > > > >
> > > > > - Empty and tautological comments are kind of garbage that reduce
> > > > > readability.
> > > > > - It's better to leave the entity undocumented, than write
> > > > > unexpressive/misleading comment.
> > > > > - Even classes may not require javadocs, e.g. simple DTOs.
> > > > >
> > > > > ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:
> > > > >
> > > > > > Thx for feedback!
> > > > > >
> > > > > > >> we have to write proper javadoc for all production classes,
> > > > including
> > > > > > internal.
> > > > > >
> > > > > > Nikolay, I cannot agree with it.
> > > > > >
> > > > > > What should be the best comment for the next fields?
> > > > > > /** */
> > > > > > private static final long serialVersionUID = 0L;
> > > > > > or
> > > > > > /** */
> > > > > > @LoggerResource
> > > > > > private IgniteLogger log;
> > > > > >
> > > > > > There are more than 8000 lines of /** */ only at the ignite-core
> > > > module (do
> > > > > > not include tests).
> > > > > >
> > > > > > Any comments will be redundant and just noise. Obvious comment
> learns
> > > > > > readers skip all comments.
> > > > > >
> > > > > >
> > > > > > >> identical distance/padding/margin between fields in a class -
> is
> > > > really
> > > > > > cool
> > > > > >
> > > > > > Vyacheslav, but we have a blank line between fields. Why is one
> blank
> > > > line
> > > > > > not enough?
> > > > > >
> > > > > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > Denis, thank you for starting this discussion!
> > > > > > >
> > > > > > > My opinion here is that having a good javadoc for every class
> and
> > > > > > > method is not feasible in the real world. I am quite curious
> to see a
> > > > > > > non-trivial project which follows it. Also, all comments and
> javadocs
> > > > > > > are prone to become misleading when code changes (human
> factor). In
> > > > my
> > > > > > > experience good method and variable names and clear code
> organization
> > > > > > > often are more helpful than javadocs.
> > > > > > >
> > > > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <
> [hidden email]
> > > > >:
> > > > > > > >
> > > > > > > > I agree that useless comments look weird in the codebase.
> > > > > > > >
> > > > > > > > But, identical distance/padding/margin between fields in a
> class -
> > > > is
> > > > > > > > really cool, and helps read the class very fast.
> > > > > > > >
> > > > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> > > > [hidden email]>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hello, Denis.
> > > > > > > > >
> > > > > > > > > Thanks for starting this discussion.
> > > > > > > > >
> > > > > > > > > I think we have to write proper javadoc for all production
> > > > classes,
> > > > > > > including internal.
> > > > > > > > > We should fix useless javadoc you provide.
> > > > > > > > > We should not accept patches without good javadocs.
> > > > > > > > >
> > > > > > > > > As for the tests, seems, we can make javadoc optional.
> > > > > > > > >
> > > > > > > > > What do you think?
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > > > > > Igniters!
> > > > > > > > > >
> > > > > > > > > > I think it's time to change coding guidelines in part of
> > > > JavaDoc
> > > > > > > comments
> > > > > > > > > > [1]:
> > > > > > > > > > > > Every method, field or initializer public, private or
> > > > protected
> > > > > > > in
> > > > > > > > > >
> > > > > > > > > > top-level,
> > > > > > > > > > > > inner or anonymous type should have at least minimal
> > > > Javadoc
> > > > > > > comments
> > > > > > > > > >
> > > > > > > > > > including
> > > > > > > > > > > > description and description of parameters using
> @param,
> > > > @return
> > > > > > > and
> > > > > > > > > >
> > > > > > > > > > @throws Javadoc tags,
> > > > > > > > > > > > where applicable.
> > > > > > > > > >
> > > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > > > > > > >
> > > > > > > > > > Why?
> > > > > > > > > >
> > > > > > > > > > /** */ - 15 matches.
> > > > > > > > > >
> > > > > > > > > > What can you get new from these comments?
> > > > > > > > > >
> > > > > > > > > > /** Periodic starvation check interval. */
> > > > > > > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ
> =
> > > > 1000 *
> > > > > > 30;
> > > > > > > > > >
> > > > > > > > > > /** Long jvm pause detector. */
> > > > > > > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > > > > > > >
> > > > > > > > > > /** Scheduler. */
> > > > > > > > > >     private IgniteScheduler scheduler;
> > > > > > > > > >
> > > > > > > > > > /** Stop guard. */
> > > > > > > > > >     private final AtomicBoolean stopGuard = new
> > > > AtomicBoolean();
> > > > > > > > > >
> > > > > > > > > > /**
> > > > > > > > > >      * @param cfg Configuration to use.
> > > > > > > > > >      * @param utilityCachePool Utility cache pool.
> > > > > > > > > >      * @param execSvc Executor service.
> > > > > > > > > >      * @param sysExecSvc System executor service.
> > > > > > > > > >      * @param stripedExecSvc Striped executor.
> > > > > > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > > > > > >      * @param mgmtExecSvc Management executor service.
> > > > > > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > > > > > >      * @param dataStreamExecSvc data stream executor
> service.
> > > > > > > > > >      * @param restExecSvc Reset executor service.
> > > > > > > > > >      * @param affExecSvc Affinity executor service.
> > > > > > > > > >      * @param idxExecSvc Indexing executor service.
> > > > > > > > > >      * @param callbackExecSvc Callback executor service.
> > > > > > > > > >      * @param qryExecSvc Query executor service.
> > > > > > > > > >      * @param schemaExecSvc Schema executor service.
> > > > > > > > > >      * @param customExecSvcs Custom named executors.
> > > > > > > > > >      * @param errHnd Error handler to use for
> notification
> > > > about
> > > > > > > startup
> > > > > > > > > > problems.
> > > > > > > > > >      * @param workerRegistry Worker registry.
> > > > > > > > > >      * @param hnd Default uncaught exception handler
> used by
> > > > thread
> > > > > > > pools.
> > > > > > > > > >      * @throws IgniteCheckedException Thrown in case of
> any
> > > > errors.
> > > > > > > > > >      */
> > > > > > > > > >     public void start(
> > > > > > > > > >         final IgniteConfiguration cfg,
> > > > > > > > > >         ExecutorService utilityCachePool,
> > > > > > > > > >         final ExecutorService execSvc,
> > > > > > > > > >         final ExecutorService svcExecSvc,
> > > > > > > > > >         final ExecutorService sysExecSvc,
> > > > > > > > > >         final StripedExecutor stripedExecSvc,
> > > > > > > > > >         ExecutorService p2pExecSvc,
> > > > > > > > > >         ExecutorService mgmtExecSvc,
> > > > > > > > > >         ExecutorService igfsExecSvc,
> > > > > > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > > > > > >         ExecutorService restExecSvc,
> > > > > > > > > >         ExecutorService affExecSvc,
> > > > > > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > > > > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > > > > > > >         ExecutorService qryExecSvc,
> > > > > > > > > >         ExecutorService schemaExecSvc,
> > > > > > > > > >         @Nullable final Map<String, ? extends
> ExecutorService>
> > > > > > > > > > customExecSvcs,
> > > > > > > > > >         GridAbsClosure errHnd,
> > > > > > > > > >         WorkersRegistry workerRegistry,
> > > > > > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > > > > > >         TimeBag startTimer
> > > > > > > > > >     )
> > > > > > > > > >
> > > > > > > > > > These comments look ugly and useless, and that is the
> main
> > > > class of
> > > > > > > core.
> > > > > > > > > > Why do they need us?
> > > > > > > > > > Let us change the coding guidelines in part of JavaDoc
> > > > comments:
> > > > > > > > > > Every method public API should have at least minimal
> Javadoc
> > > > > > > comments,
> > > > > > > > > > including description and description of parameters using
> > > > @param,
> > > > > > > @return,
> > > > > > > > > > and @throws Javadoc tags,
> > > > > > > > > > where applicable.
> > > > > > > > > > For internal API, JavaDoc comments should be optional.
> It's up
> > > > to a
> > > > > > > > > > contributor or reviewer.
> > > > > > > > > >
> > > > > > > > > > What are you think?
> > > > > > > > > >
> > > > > > > > > > 1.
> > > > > > > > > >
> > > > > > >
> > > > > >
> > > >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Best Regards, Vyacheslav D.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Best regards,
> > > > > > > Ivan Pavlukhin
> > > > > > >
> > > > > >
> > > >
> >
> >
> >
> > --
> > Best regards,
> > Ivan Pavlukhin
>

[Pavel Kovalenko]

I can agree that some part of javadocs we have is useless. It relates to
DTOs, getters/setters without side-effects, short self-descriptive methods.
In an ideal world, proper modularization of architecture, leading to
KISS/SOLID/DRY/etc. principles, writing self-documented code should result
in javadocs disappearing, as they become not needed.
We live in a not ideal world. We don't have good architecture and can't
always lead to mentioned principles, because we need sometimes sacrifice
readability for optimization, fixing a critical bug, etc.
I think that API of our core internal things like PageMemory, WAL, all
existing managers and processors should be as well documented as possible.
If a developer uses some module / manager / processor without looking
inside, reading the only description of public methods, it's a good sign
that this part of the functionality is well documented.
Internal implementation should be also clear for a developer who likes to
make a change inside it. Every optimization, avoiding race-condition, not
obvious thing and especially crutch must be documented as detailed as
possible.
Documentation should be a result of a proper code review. If a reviewer has
questions regarding any code line it should be either refactored to make
this thing obvious or well documented.
If a class or method is self-documented and obvious there is no need to
document it anyway.
if each person takes the code review as seriously as possible,
documentation and code will be better automatically.
Mandatory documentation in places where it's really not needed looks like a
burden. A developer will avoid write it properly everywhere or do it "just
for check" and this will influence on documentation with the negative side.
Flexible approach with mandatory / optional javadocs with good code review
will result in readability improvement overall.


чт, 8 авг. 2019 г. в 17:52, Maxim Muzafarov <[hidden email]>:

> Ivan,
>
> It is not a problem to check Javadocs at the moment code syle checking
> performed, but do we really need this? And the human-factor you
> mentioned above is also related to the `self-descriptive` names. I
> assume, that someone now is desiring to use single-letter variables
> and single-letter class names to save space an time. We will always
> have such an opinion race.
>
> I'd prefer to leave the current situation with Javadoc as it is and
> just to ask to apply the patch [1][2]. Can you? :-)
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12051
> [2] https://github.com/apache/ignite/pull/6760
>
> On Thu, 8 Aug 2019 at 17:24, Павлухин Иван <[hidden email]> wrote:
> >
> > Maxim,
> >
> > My main concern here is a human factor. Humans are usually not so good
> > in keeping documentation always in sync with a code. Examples from an
> > actual PR:
> > /**
> > * @param nodeId The remote node id.
> > * @param channel The channel to notify listeners with.
> > */
> > private void onChannelOpened0(UUID nodeId, GridIoMessage initMsg,
> > Channel channel)
> >
> > First, there is a mismatch between number of parameters in javadoc and
> > code. Second, e.g. nodeId name can be made self-descriptive rmtNodeId
> > name.
> >
> > Mandatory javadocs do not imply good javadocs. Good javadocs do not
> > imply javadocs for every method and field. For me, mandatory and good
> > javadocs are like communism. Sounds quite nice in theory, but not
> > feasible in practice.
> >
> > чт, 8 авг. 2019 г. в 16:55, Denis Garus <[hidden email]>:
> > >
> > > Thank you, Maxim.
> > >
> > > >>On what we are trying to save by making Javadoc optional?
> > >
> > > Space and time.
> > >
> > > I doubt that we need a verbal description of what private method make.
> > > Why we need it if we just can read the code?
> > >
> > > Bright examples:
> > >
> > > /**
> > > * @param helper Helper to attach to kernal context.
> > > */
> > > private void addHelper(Object helper) {
> > >     ctx.addHelper(helper);
> > > }
> > >
> > > /**
> > > * Gets "on" or "off" string for given boolean value.
> > > *
> > > * @param b Boolean value to convert.
> > > * @return Result string.
> > > */
> > > private String onOff(boolean b) {
> > >     return b ? "on" : "off";
> > > }
> > >
> > > You have to support both code of method and their JavaDoc description,
> but
> > > it doesn't get any sake.
> > >
> > > >>Let's just help them to read the source code.
> > >
> > > But at the same time, public method IgniteKernal#start doesn't have any
> > > description except enumeration of attributes with apparent notes.
> > > Maybe to give it a verbal description needs one or two pages of the
> screen.
> > > Does it make sense?
> > >
> > > чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov <[hidden email]>:
> > >
> > > > Igniters,
> > > >
> > > > I'm -1 with making Javadoc optional (except tests).
> > > >
> > > > Here is my patch [1] with PR [2] which fixes all the Javadoc comments
> > > > for the IgniteKernal class mentioned above. Please, take a look. The
> > > > review is very appreciated.
> > > >
> > > > On what we are trying to save by making Javadoc optional? In my
> humble
> > > > opinion - Javadoc comments are mostly concern users who debug the
> > > > Ignite when they have faced with some unhandled exception or related
> > > > to the community members with an average involvement (produces a few
> > > > small patches during the year) but not to the experienced one. Let's
> > > > just help them to read the source code.
> > > >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > > [2] https://github.com/apache/ignite/pull/6760
> > > >
> > > >
> > > >
> > > > On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <[hidden email]>
> wrote:
> > > > >
> > > > > +1 for making javadoc comments optional.
> > > > >
> > > > > - Empty and tautological comments are kind of garbage that reduce
> > > > > readability.
> > > > > - It's better to leave the entity undocumented, than write
> > > > > unexpressive/misleading comment.
> > > > > - Even classes may not require javadocs, e.g. simple DTOs.
> > > > >
> > > > > ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:
> > > > >
> > > > > > Thx for feedback!
> > > > > >
> > > > > > >> we have to write proper javadoc for all production classes,
> > > > including
> > > > > > internal.
> > > > > >
> > > > > > Nikolay, I cannot agree with it.
> > > > > >
> > > > > > What should be the best comment for the next fields?
> > > > > > /** */
> > > > > > private static final long serialVersionUID = 0L;
> > > > > > or
> > > > > > /** */
> > > > > > @LoggerResource
> > > > > > private IgniteLogger log;
> > > > > >
> > > > > > There are more than 8000 lines of /** */ only at the ignite-core
> > > > module (do
> > > > > > not include tests).
> > > > > >
> > > > > > Any comments will be redundant and just noise. Obvious comment
> learns
> > > > > > readers skip all comments.
> > > > > >
> > > > > >
> > > > > > >> identical distance/padding/margin between fields in a class -
> is
> > > > really
> > > > > > cool
> > > > > >
> > > > > > Vyacheslav, but we have a blank line between fields. Why is one
> blank
> > > > line
> > > > > > not enough?
> > > > > >
> > > > > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > Denis, thank you for starting this discussion!
> > > > > > >
> > > > > > > My opinion here is that having a good javadoc for every class
> and
> > > > > > > method is not feasible in the real world. I am quite curious
> to see a
> > > > > > > non-trivial project which follows it. Also, all comments and
> javadocs
> > > > > > > are prone to become misleading when code changes (human
> factor). In
> > > > my
> > > > > > > experience good method and variable names and clear code
> organization
> > > > > > > often are more helpful than javadocs.
> > > > > > >
> > > > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <
> [hidden email]
> > > > >:
> > > > > > > >
> > > > > > > > I agree that useless comments look weird in the codebase.
> > > > > > > >
> > > > > > > > But, identical distance/padding/margin between fields in a
> class -
> > > > is
> > > > > > > > really cool, and helps read the class very fast.
> > > > > > > >
> > > > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> > > > [hidden email]>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hello, Denis.
> > > > > > > > >
> > > > > > > > > Thanks for starting this discussion.
> > > > > > > > >
> > > > > > > > > I think we have to write proper javadoc for all production
> > > > classes,
> > > > > > > including internal.
> > > > > > > > > We should fix useless javadoc you provide.
> > > > > > > > > We should not accept patches without good javadocs.
> > > > > > > > >
> > > > > > > > > As for the tests, seems, we can make javadoc optional.
> > > > > > > > >
> > > > > > > > > What do you think?
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > > > > > Igniters!
> > > > > > > > > >
> > > > > > > > > > I think it's time to change coding guidelines in part of
> > > > JavaDoc
> > > > > > > comments
> > > > > > > > > > [1]:
> > > > > > > > > > > > Every method, field or initializer public, private or
> > > > protected
> > > > > > > in
> > > > > > > > > >
> > > > > > > > > > top-level,
> > > > > > > > > > > > inner or anonymous type should have at least minimal
> > > > Javadoc
> > > > > > > comments
> > > > > > > > > >
> > > > > > > > > > including
> > > > > > > > > > > > description and description of parameters using
> @param,
> > > > @return
> > > > > > > and
> > > > > > > > > >
> > > > > > > > > > @throws Javadoc tags,
> > > > > > > > > > > > where applicable.
> > > > > > > > > >
> > > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > > > > > > >
> > > > > > > > > > Why?
> > > > > > > > > >
> > > > > > > > > > /** */ - 15 matches.
> > > > > > > > > >
> > > > > > > > > > What can you get new from these comments?
> > > > > > > > > >
> > > > > > > > > > /** Periodic starvation check interval. */
> > > > > > > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ
> =
> > > > 1000 *
> > > > > > 30;
> > > > > > > > > >
> > > > > > > > > > /** Long jvm pause detector. */
> > > > > > > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > > > > > > >
> > > > > > > > > > /** Scheduler. */
> > > > > > > > > >     private IgniteScheduler scheduler;
> > > > > > > > > >
> > > > > > > > > > /** Stop guard. */
> > > > > > > > > >     private final AtomicBoolean stopGuard = new
> > > > AtomicBoolean();
> > > > > > > > > >
> > > > > > > > > > /**
> > > > > > > > > >      * @param cfg Configuration to use.
> > > > > > > > > >      * @param utilityCachePool Utility cache pool.
> > > > > > > > > >      * @param execSvc Executor service.
> > > > > > > > > >      * @param sysExecSvc System executor service.
> > > > > > > > > >      * @param stripedExecSvc Striped executor.
> > > > > > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > > > > > >      * @param mgmtExecSvc Management executor service.
> > > > > > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > > > > > >      * @param dataStreamExecSvc data stream executor
> service.
> > > > > > > > > >      * @param restExecSvc Reset executor service.
> > > > > > > > > >      * @param affExecSvc Affinity executor service.
> > > > > > > > > >      * @param idxExecSvc Indexing executor service.
> > > > > > > > > >      * @param callbackExecSvc Callback executor service.
> > > > > > > > > >      * @param qryExecSvc Query executor service.
> > > > > > > > > >      * @param schemaExecSvc Schema executor service.
> > > > > > > > > >      * @param customExecSvcs Custom named executors.
> > > > > > > > > >      * @param errHnd Error handler to use for
> notification
> > > > about
> > > > > > > startup
> > > > > > > > > > problems.
> > > > > > > > > >      * @param workerRegistry Worker registry.
> > > > > > > > > >      * @param hnd Default uncaught exception handler
> used by
> > > > thread
> > > > > > > pools.
> > > > > > > > > >      * @throws IgniteCheckedException Thrown in case of
> any
> > > > errors.
> > > > > > > > > >      */
> > > > > > > > > >     public void start(
> > > > > > > > > >         final IgniteConfiguration cfg,
> > > > > > > > > >         ExecutorService utilityCachePool,
> > > > > > > > > >         final ExecutorService execSvc,
> > > > > > > > > >         final ExecutorService svcExecSvc,
> > > > > > > > > >         final ExecutorService sysExecSvc,
> > > > > > > > > >         final StripedExecutor stripedExecSvc,
> > > > > > > > > >         ExecutorService p2pExecSvc,
> > > > > > > > > >         ExecutorService mgmtExecSvc,
> > > > > > > > > >         ExecutorService igfsExecSvc,
> > > > > > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > > > > > >         ExecutorService restExecSvc,
> > > > > > > > > >         ExecutorService affExecSvc,
> > > > > > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > > > > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > > > > > > >         ExecutorService qryExecSvc,
> > > > > > > > > >         ExecutorService schemaExecSvc,
> > > > > > > > > >         @Nullable final Map<String, ? extends
> ExecutorService>
> > > > > > > > > > customExecSvcs,
> > > > > > > > > >         GridAbsClosure errHnd,
> > > > > > > > > >         WorkersRegistry workerRegistry,
> > > > > > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > > > > > >         TimeBag startTimer
> > > > > > > > > >     )
> > > > > > > > > >
> > > > > > > > > > These comments look ugly and useless, and that is the
> main
> > > > class of
> > > > > > > core.
> > > > > > > > > > Why do they need us?
> > > > > > > > > > Let us change the coding guidelines in part of JavaDoc
> > > > comments:
> > > > > > > > > > Every method public API should have at least minimal
> Javadoc
> > > > > > > comments,
> > > > > > > > > > including description and description of parameters using
> > > > @param,
> > > > > > > @return,
> > > > > > > > > > and @throws Javadoc tags,
> > > > > > > > > > where applicable.
> > > > > > > > > > For internal API, JavaDoc comments should be optional.
> It's up
> > > > to a
> > > > > > > > > > contributor or reviewer.
> > > > > > > > > >
> > > > > > > > > > What are you think?
> > > > > > > > > >
> > > > > > > > > > 1.
> > > > > > > > > >
> > > > > > >
> > > > > >
> > > >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Best Regards, Vyacheslav D.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Best regards,
> > > > > > > Ivan Pavlukhin
> > > > > > >
> > > > > >
> > > >
> >
> >
> >
> > --
> > Best regards,
> > Ivan Pavlukhin
>

[Ivan Pavlukhin]

Maxim,

> I'd prefer to leave the current situation with Javadoc as it is and just to ask to apply the patch [1][2]. Can you? :-)

You caught me. I left my comments for that PR

.Pavel and all,

> I think that API of our core internal things like PageMemory, WAL, all existing managers and processors should be as well documented as possible.

No doubts here.

> Documentation should be a result of a proper code review.

I suppose it does not mean that documentation should be written after
a review. I suppose it means that we should not have a poor
documentation *after* a review. Overall, Pavel's last message conforms
well with my opinion on the subject.

чт, 8 авг. 2019 г. в 18:34, Pavel Kovalenko <[hidden email]>:

>
> I can agree that some part of javadocs we have is useless. It relates to
> DTOs, getters/setters without side-effects, short self-descriptive methods.
> In an ideal world, proper modularization of architecture, leading to
> KISS/SOLID/DRY/etc. principles, writing self-documented code should result
> in javadocs disappearing, as they become not needed.
> We live in a not ideal world. We don't have good architecture and can't
> always lead to mentioned principles, because we need sometimes sacrifice
> readability for optimization, fixing a critical bug, etc.
> I think that API of our core internal things like PageMemory, WAL, all
> existing managers and processors should be as well documented as possible.
> If a developer uses some module / manager / processor without looking
> inside, reading the only description of public methods, it's a good sign
> that this part of the functionality is well documented.
> Internal implementation should be also clear for a developer who likes to
> make a change inside it. Every optimization, avoiding race-condition, not
> obvious thing and especially crutch must be documented as detailed as
> possible.
> Documentation should be a result of a proper code review. If a reviewer has
> questions regarding any code line it should be either refactored to make
> this thing obvious or well documented.
> If a class or method is self-documented and obvious there is no need to
> document it anyway.
> if each person takes the code review as seriously as possible,
> documentation and code will be better automatically.
> Mandatory documentation in places where it's really not needed looks like a
> burden. A developer will avoid write it properly everywhere or do it "just
> for check" and this will influence on documentation with the negative side.
> Flexible approach with mandatory / optional javadocs with good code review
> will result in readability improvement overall.
>
>
> чт, 8 авг. 2019 г. в 17:52, Maxim Muzafarov <[hidden email]>:
>
> > Ivan,
> >
> > It is not a problem to check Javadocs at the moment code syle checking
> > performed, but do we really need this? And the human-factor you
> > mentioned above is also related to the `self-descriptive` names. I
> > assume, that someone now is desiring to use single-letter variables
> > and single-letter class names to save space an time. We will always
> > have such an opinion race.
> >
> > I'd prefer to leave the current situation with Javadoc as it is and
> > just to ask to apply the patch [1][2]. Can you? :-)
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > [2] https://github.com/apache/ignite/pull/6760
> >
> > On Thu, 8 Aug 2019 at 17:24, Павлухин Иван <[hidden email]> wrote:
> > >
> > > Maxim,
> > >
> > > My main concern here is a human factor. Humans are usually not so good
> > > in keeping documentation always in sync with a code. Examples from an
> > > actual PR:
> > > /**
> > > * @param nodeId The remote node id.
> > > * @param channel The channel to notify listeners with.
> > > */
> > > private void onChannelOpened0(UUID nodeId, GridIoMessage initMsg,
> > > Channel channel)
> > >
> > > First, there is a mismatch between number of parameters in javadoc and
> > > code. Second, e.g. nodeId name can be made self-descriptive rmtNodeId
> > > name.
> > >
> > > Mandatory javadocs do not imply good javadocs. Good javadocs do not
> > > imply javadocs for every method and field. For me, mandatory and good
> > > javadocs are like communism. Sounds quite nice in theory, but not
> > > feasible in practice.
> > >
> > > чт, 8 авг. 2019 г. в 16:55, Denis Garus <[hidden email]>:
> > > >
> > > > Thank you, Maxim.
> > > >
> > > > >>On what we are trying to save by making Javadoc optional?
> > > >
> > > > Space and time.
> > > >
> > > > I doubt that we need a verbal description of what private method make.
> > > > Why we need it if we just can read the code?
> > > >
> > > > Bright examples:
> > > >
> > > > /**
> > > > * @param helper Helper to attach to kernal context.
> > > > */
> > > > private void addHelper(Object helper) {
> > > >     ctx.addHelper(helper);
> > > > }
> > > >
> > > > /**
> > > > * Gets "on" or "off" string for given boolean value.
> > > > *
> > > > * @param b Boolean value to convert.
> > > > * @return Result string.
> > > > */
> > > > private String onOff(boolean b) {
> > > >     return b ? "on" : "off";
> > > > }
> > > >
> > > > You have to support both code of method and their JavaDoc description,
> > but
> > > > it doesn't get any sake.
> > > >
> > > > >>Let's just help them to read the source code.
> > > >
> > > > But at the same time, public method IgniteKernal#start doesn't have any
> > > > description except enumeration of attributes with apparent notes.
> > > > Maybe to give it a verbal description needs one or two pages of the
> > screen.
> > > > Does it make sense?
> > > >
> > > > чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov <[hidden email]>:
> > > >
> > > > > Igniters,
> > > > >
> > > > > I'm -1 with making Javadoc optional (except tests).
> > > > >
> > > > > Here is my patch [1] with PR [2] which fixes all the Javadoc comments
> > > > > for the IgniteKernal class mentioned above. Please, take a look. The
> > > > > review is very appreciated.
> > > > >
> > > > > On what we are trying to save by making Javadoc optional? In my
> > humble
> > > > > opinion - Javadoc comments are mostly concern users who debug the
> > > > > Ignite when they have faced with some unhandled exception or related
> > > > > to the community members with an average involvement (produces a few
> > > > > small patches during the year) but not to the experienced one. Let's
> > > > > just help them to read the source code.
> > > > >
> > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > > > [2] https://github.com/apache/ignite/pull/6760
> > > > >
> > > > >
> > > > >
> > > > > On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <[hidden email]>
> > wrote:
> > > > > >
> > > > > > +1 for making javadoc comments optional.
> > > > > >
> > > > > > - Empty and tautological comments are kind of garbage that reduce
> > > > > > readability.
> > > > > > - It's better to leave the entity undocumented, than write
> > > > > > unexpressive/misleading comment.
> > > > > > - Even classes may not require javadocs, e.g. simple DTOs.
> > > > > >
> > > > > > ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:
> > > > > >
> > > > > > > Thx for feedback!
> > > > > > >
> > > > > > > >> we have to write proper javadoc for all production classes,
> > > > > including
> > > > > > > internal.
> > > > > > >
> > > > > > > Nikolay, I cannot agree with it.
> > > > > > >
> > > > > > > What should be the best comment for the next fields?
> > > > > > > /** */
> > > > > > > private static final long serialVersionUID = 0L;
> > > > > > > or
> > > > > > > /** */
> > > > > > > @LoggerResource
> > > > > > > private IgniteLogger log;
> > > > > > >
> > > > > > > There are more than 8000 lines of /** */ only at the ignite-core
> > > > > module (do
> > > > > > > not include tests).
> > > > > > >
> > > > > > > Any comments will be redundant and just noise. Obvious comment
> > learns
> > > > > > > readers skip all comments.
> > > > > > >
> > > > > > >
> > > > > > > >> identical distance/padding/margin between fields in a class -
> > is
> > > > > really
> > > > > > > cool
> > > > > > >
> > > > > > > Vyacheslav, but we have a blank line between fields. Why is one
> > blank
> > > > > line
> > > > > > > not enough?
> > > > > > >
> > > > > > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <[hidden email]>:
> > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > Denis, thank you for starting this discussion!
> > > > > > > >
> > > > > > > > My opinion here is that having a good javadoc for every class
> > and
> > > > > > > > method is not feasible in the real world. I am quite curious
> > to see a
> > > > > > > > non-trivial project which follows it. Also, all comments and
> > javadocs
> > > > > > > > are prone to become misleading when code changes (human
> > factor). In
> > > > > my
> > > > > > > > experience good method and variable names and clear code
> > organization
> > > > > > > > often are more helpful than javadocs.
> > > > > > > >
> > > > > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <
> > [hidden email]
> > > > > >:
> > > > > > > > >
> > > > > > > > > I agree that useless comments look weird in the codebase.
> > > > > > > > >
> > > > > > > > > But, identical distance/padding/margin between fields in a
> > class -
> > > > > is
> > > > > > > > > really cool, and helps read the class very fast.
> > > > > > > > >
> > > > > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> > > > > [hidden email]>
> > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hello, Denis.
> > > > > > > > > >
> > > > > > > > > > Thanks for starting this discussion.
> > > > > > > > > >
> > > > > > > > > > I think we have to write proper javadoc for all production
> > > > > classes,
> > > > > > > > including internal.
> > > > > > > > > > We should fix useless javadoc you provide.
> > > > > > > > > > We should not accept patches without good javadocs.
> > > > > > > > > >
> > > > > > > > > > As for the tests, seems, we can make javadoc optional.
> > > > > > > > > >
> > > > > > > > > > What do you think?
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > > > > > > Igniters!
> > > > > > > > > > >
> > > > > > > > > > > I think it's time to change coding guidelines in part of
> > > > > JavaDoc
> > > > > > > > comments
> > > > > > > > > > > [1]:
> > > > > > > > > > > > > Every method, field or initializer public, private or
> > > > > protected
> > > > > > > > in
> > > > > > > > > > >
> > > > > > > > > > > top-level,
> > > > > > > > > > > > > inner or anonymous type should have at least minimal
> > > > > Javadoc
> > > > > > > > comments
> > > > > > > > > > >
> > > > > > > > > > > including
> > > > > > > > > > > > > description and description of parameters using
> > @param,
> > > > > @return
> > > > > > > > and
> > > > > > > > > > >
> > > > > > > > > > > @throws Javadoc tags,
> > > > > > > > > > > > > where applicable.
> > > > > > > > > > >
> > > > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > > > > > > > >
> > > > > > > > > > > Why?
> > > > > > > > > > >
> > > > > > > > > > > /** */ - 15 matches.
> > > > > > > > > > >
> > > > > > > > > > > What can you get new from these comments?
> > > > > > > > > > >
> > > > > > > > > > > /** Periodic starvation check interval. */
> > > > > > > > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ
> > =
> > > > > 1000 *
> > > > > > > 30;
> > > > > > > > > > >
> > > > > > > > > > > /** Long jvm pause detector. */
> > > > > > > > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > > > > > > > >
> > > > > > > > > > > /** Scheduler. */
> > > > > > > > > > >     private IgniteScheduler scheduler;
> > > > > > > > > > >
> > > > > > > > > > > /** Stop guard. */
> > > > > > > > > > >     private final AtomicBoolean stopGuard = new
> > > > > AtomicBoolean();
> > > > > > > > > > >
> > > > > > > > > > > /**
> > > > > > > > > > >      * @param cfg Configuration to use.
> > > > > > > > > > >      * @param utilityCachePool Utility cache pool.
> > > > > > > > > > >      * @param execSvc Executor service.
> > > > > > > > > > >      * @param sysExecSvc System executor service.
> > > > > > > > > > >      * @param stripedExecSvc Striped executor.
> > > > > > > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > > > > > > >      * @param mgmtExecSvc Management executor service.
> > > > > > > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > > > > > > >      * @param dataStreamExecSvc data stream executor
> > service.
> > > > > > > > > > >      * @param restExecSvc Reset executor service.
> > > > > > > > > > >      * @param affExecSvc Affinity executor service.
> > > > > > > > > > >      * @param idxExecSvc Indexing executor service.
> > > > > > > > > > >      * @param callbackExecSvc Callback executor service.
> > > > > > > > > > >      * @param qryExecSvc Query executor service.
> > > > > > > > > > >      * @param schemaExecSvc Schema executor service.
> > > > > > > > > > >      * @param customExecSvcs Custom named executors.
> > > > > > > > > > >      * @param errHnd Error handler to use for
> > notification
> > > > > about
> > > > > > > > startup
> > > > > > > > > > > problems.
> > > > > > > > > > >      * @param workerRegistry Worker registry.
> > > > > > > > > > >      * @param hnd Default uncaught exception handler
> > used by
> > > > > thread
> > > > > > > > pools.
> > > > > > > > > > >      * @throws IgniteCheckedException Thrown in case of
> > any
> > > > > errors.
> > > > > > > > > > >      */
> > > > > > > > > > >     public void start(
> > > > > > > > > > >         final IgniteConfiguration cfg,
> > > > > > > > > > >         ExecutorService utilityCachePool,
> > > > > > > > > > >         final ExecutorService execSvc,
> > > > > > > > > > >         final ExecutorService svcExecSvc,
> > > > > > > > > > >         final ExecutorService sysExecSvc,
> > > > > > > > > > >         final StripedExecutor stripedExecSvc,
> > > > > > > > > > >         ExecutorService p2pExecSvc,
> > > > > > > > > > >         ExecutorService mgmtExecSvc,
> > > > > > > > > > >         ExecutorService igfsExecSvc,
> > > > > > > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > > > > > > >         ExecutorService restExecSvc,
> > > > > > > > > > >         ExecutorService affExecSvc,
> > > > > > > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > > > > > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > > > > > > > >         ExecutorService qryExecSvc,
> > > > > > > > > > >         ExecutorService schemaExecSvc,
> > > > > > > > > > >         @Nullable final Map<String, ? extends
> > ExecutorService>
> > > > > > > > > > > customExecSvcs,
> > > > > > > > > > >         GridAbsClosure errHnd,
> > > > > > > > > > >         WorkersRegistry workerRegistry,
> > > > > > > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > > > > > > >         TimeBag startTimer
> > > > > > > > > > >     )
> > > > > > > > > > >
> > > > > > > > > > > These comments look ugly and useless, and that is the
> > main
> > > > > class of
> > > > > > > > core.
> > > > > > > > > > > Why do they need us?
> > > > > > > > > > > Let us change the coding guidelines in part of JavaDoc
> > > > > comments:
> > > > > > > > > > > Every method public API should have at least minimal
> > Javadoc
> > > > > > > > comments,
> > > > > > > > > > > including description and description of parameters using
> > > > > @param,
> > > > > > > > @return,
> > > > > > > > > > > and @throws Javadoc tags,
> > > > > > > > > > > where applicable.
> > > > > > > > > > > For internal API, JavaDoc comments should be optional.
> > It's up
> > > > > to a
> > > > > > > > > > > contributor or reviewer.
> > > > > > > > > > >
> > > > > > > > > > > What are you think?
> > > > > > > > > > >
> > > > > > > > > > > 1.
> > > > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Best Regards, Vyacheslav D.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Best regards,
> > > > > > > > Ivan Pavlukhin
> > > > > > > >
> > > > > > >
> > > > >
> > >
> > >
> > >
> > > --
> > > Best regards,
> > > Ivan Pavlukhin
> >

--
Best regards,
Ivan Pavlukhin

[Anton Vinogradov-2]

My +1 to optional Javadoc at private methods, fields, and classes.
The reviewer should guaranty that everything is clear.
But, in case everything is clear without redundant Javadoc there is no need
to have it.

So, my proposal is to allow /** */ for non-public fields and methods (to
keep readability between documented and obvious fields/methods).
Also, I'd like to propose to have only non-obvious params explained at
non-public method's Javadoc.

On Sun, Aug 11, 2019 at 9:13 AM Павлухин Иван <[hidden email]> wrote:

> Maxim,
>
> > I'd prefer to leave the current situation with Javadoc as it is and just
> to ask to apply the patch [1][2]. Can you? :-)
>
> You caught me. I left my comments for that PR
>
> .Pavel and all,
>
> > I think that API of our core internal things like PageMemory, WAL, all
> existing managers and processors should be as well documented as possible.
>
> No doubts here.
>
> > Documentation should be a result of a proper code review.
>
> I suppose it does not mean that documentation should be written after
> a review. I suppose it means that we should not have a poor
> documentation *after* a review. Overall, Pavel's last message conforms
> well with my opinion on the subject.
>
> чт, 8 авг. 2019 г. в 18:34, Pavel Kovalenko <[hidden email]>:
> >
> > I can agree that some part of javadocs we have is useless. It relates to
> > DTOs, getters/setters without side-effects, short self-descriptive
> methods.
> > In an ideal world, proper modularization of architecture, leading to
> > KISS/SOLID/DRY/etc. principles, writing self-documented code should
> result
> > in javadocs disappearing, as they become not needed.
> > We live in a not ideal world. We don't have good architecture and can't
> > always lead to mentioned principles, because we need sometimes sacrifice
> > readability for optimization, fixing a critical bug, etc.
> > I think that API of our core internal things like PageMemory, WAL, all
> > existing managers and processors should be as well documented as
> possible.
> > If a developer uses some module / manager / processor without looking
> > inside, reading the only description of public methods, it's a good sign
> > that this part of the functionality is well documented.
> > Internal implementation should be also clear for a developer who likes to
> > make a change inside it. Every optimization, avoiding race-condition, not
> > obvious thing and especially crutch must be documented as detailed as
> > possible.
> > Documentation should be a result of a proper code review. If a reviewer
> has
> > questions regarding any code line it should be either refactored to make
> > this thing obvious or well documented.
> > If a class or method is self-documented and obvious there is no need to
> > document it anyway.
> > if each person takes the code review as seriously as possible,
> > documentation and code will be better automatically.
> > Mandatory documentation in places where it's really not needed looks
> like a
> > burden. A developer will avoid write it properly everywhere or do it
> "just
> > for check" and this will influence on documentation with the negative
> side.
> > Flexible approach with mandatory / optional javadocs with good code
> review
> > will result in readability improvement overall.
> >
> >
> > чт, 8 авг. 2019 г. в 17:52, Maxim Muzafarov <[hidden email]>:
> >
> > > Ivan,
> > >
> > > It is not a problem to check Javadocs at the moment code syle checking
> > > performed, but do we really need this? And the human-factor you
> > > mentioned above is also related to the `self-descriptive` names. I
> > > assume, that someone now is desiring to use single-letter variables
> > > and single-letter class names to save space an time. We will always
> > > have such an opinion race.
> > >
> > > I'd prefer to leave the current situation with Javadoc as it is and
> > > just to ask to apply the patch [1][2]. Can you? :-)
> > >
> > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > [2] https://github.com/apache/ignite/pull/6760
> > >
> > > On Thu, 8 Aug 2019 at 17:24, Павлухин Иван <[hidden email]>
> wrote:
> > > >
> > > > Maxim,
> > > >
> > > > My main concern here is a human factor. Humans are usually not so
> good
> > > > in keeping documentation always in sync with a code. Examples from an
> > > > actual PR:
> > > > /**
> > > > * @param nodeId The remote node id.
> > > > * @param channel The channel to notify listeners with.
> > > > */
> > > > private void onChannelOpened0(UUID nodeId, GridIoMessage initMsg,
> > > > Channel channel)
> > > >
> > > > First, there is a mismatch between number of parameters in javadoc
> and
> > > > code. Second, e.g. nodeId name can be made self-descriptive rmtNodeId
> > > > name.
> > > >
> > > > Mandatory javadocs do not imply good javadocs. Good javadocs do not
> > > > imply javadocs for every method and field. For me, mandatory and good
> > > > javadocs are like communism. Sounds quite nice in theory, but not
> > > > feasible in practice.
> > > >
> > > > чт, 8 авг. 2019 г. в 16:55, Denis Garus <[hidden email]>:
> > > > >
> > > > > Thank you, Maxim.
> > > > >
> > > > > >>On what we are trying to save by making Javadoc optional?
> > > > >
> > > > > Space and time.
> > > > >
> > > > > I doubt that we need a verbal description of what private method
> make.
> > > > > Why we need it if we just can read the code?
> > > > >
> > > > > Bright examples:
> > > > >
> > > > > /**
> > > > > * @param helper Helper to attach to kernal context.
> > > > > */
> > > > > private void addHelper(Object helper) {
> > > > >     ctx.addHelper(helper);
> > > > > }
> > > > >
> > > > > /**
> > > > > * Gets "on" or "off" string for given boolean value.
> > > > > *
> > > > > * @param b Boolean value to convert.
> > > > > * @return Result string.
> > > > > */
> > > > > private String onOff(boolean b) {
> > > > >     return b ? "on" : "off";
> > > > > }
> > > > >
> > > > > You have to support both code of method and their JavaDoc
> description,
> > > but
> > > > > it doesn't get any sake.
> > > > >
> > > > > >>Let's just help them to read the source code.
> > > > >
> > > > > But at the same time, public method IgniteKernal#start doesn't
> have any
> > > > > description except enumeration of attributes with apparent notes.
> > > > > Maybe to give it a verbal description needs one or two pages of the
> > > screen.
> > > > > Does it make sense?
> > > > >
> > > > > чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov <[hidden email]>:
> > > > >
> > > > > > Igniters,
> > > > > >
> > > > > > I'm -1 with making Javadoc optional (except tests).
> > > > > >
> > > > > > Here is my patch [1] with PR [2] which fixes all the Javadoc
> comments
> > > > > > for the IgniteKernal class mentioned above. Please, take a look.
> The
> > > > > > review is very appreciated.
> > > > > >
> > > > > > On what we are trying to save by making Javadoc optional? In my
> > > humble
> > > > > > opinion - Javadoc comments are mostly concern users who debug the
> > > > > > Ignite when they have faced with some unhandled exception or
> related
> > > > > > to the community members with an average involvement (produces a
> few
> > > > > > small patches during the year) but not to the experienced one.
> Let's
> > > > > > just help them to read the source code.
> > > > > >
> > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > > > > [2] https://github.com/apache/ignite/pull/6760
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <[hidden email]
> >
> > > wrote:
> > > > > > >
> > > > > > > +1 for making javadoc comments optional.
> > > > > > >
> > > > > > > - Empty and tautological comments are kind of garbage that
> reduce
> > > > > > > readability.
> > > > > > > - It's better to leave the entity undocumented, than write
> > > > > > > unexpressive/misleading comment.
> > > > > > > - Even classes may not require javadocs, e.g. simple DTOs.
> > > > > > >
> > > > > > > ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:
> > > > > > >
> > > > > > > > Thx for feedback!
> > > > > > > >
> > > > > > > > >> we have to write proper javadoc for all production
> classes,
> > > > > > including
> > > > > > > > internal.
> > > > > > > >
> > > > > > > > Nikolay, I cannot agree with it.
> > > > > > > >
> > > > > > > > What should be the best comment for the next fields?
> > > > > > > > /** */
> > > > > > > > private static final long serialVersionUID = 0L;
> > > > > > > > or
> > > > > > > > /** */
> > > > > > > > @LoggerResource
> > > > > > > > private IgniteLogger log;
> > > > > > > >
> > > > > > > > There are more than 8000 lines of /** */ only at the
> ignite-core
> > > > > > module (do
> > > > > > > > not include tests).
> > > > > > > >
> > > > > > > > Any comments will be redundant and just noise. Obvious
> comment
> > > learns
> > > > > > > > readers skip all comments.
> > > > > > > >
> > > > > > > >
> > > > > > > > >> identical distance/padding/margin between fields in a
> class -
> > > is
> > > > > > really
> > > > > > > > cool
> > > > > > > >
> > > > > > > > Vyacheslav, but we have a blank line between fields. Why is
> one
> > > blank
> > > > > > line
> > > > > > > > not enough?
> > > > > > > >
> > > > > > > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <
> [hidden email]>:
> > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > Denis, thank you for starting this discussion!
> > > > > > > > >
> > > > > > > > > My opinion here is that having a good javadoc for every
> class
> > > and
> > > > > > > > > method is not feasible in the real world. I am quite
> curious
> > > to see a
> > > > > > > > > non-trivial project which follows it. Also, all comments
> and
> > > javadocs
> > > > > > > > > are prone to become misleading when code changes (human
> > > factor). In
> > > > > > my
> > > > > > > > > experience good method and variable names and clear code
> > > organization
> > > > > > > > > often are more helpful than javadocs.
> > > > > > > > >
> > > > > > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <
> > > [hidden email]
> > > > > > >:
> > > > > > > > > >
> > > > > > > > > > I agree that useless comments look weird in the codebase.
> > > > > > > > > >
> > > > > > > > > > But, identical distance/padding/margin between fields in
> a
> > > class -
> > > > > > is
> > > > > > > > > > really cool, and helps read the class very fast.
> > > > > > > > > >
> > > > > > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> > > > > > [hidden email]>
> > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hello, Denis.
> > > > > > > > > > >
> > > > > > > > > > > Thanks for starting this discussion.
> > > > > > > > > > >
> > > > > > > > > > > I think we have to write proper javadoc for all
> production
> > > > > > classes,
> > > > > > > > > including internal.
> > > > > > > > > > > We should fix useless javadoc you provide.
> > > > > > > > > > > We should not accept patches without good javadocs.
> > > > > > > > > > >
> > > > > > > > > > > As for the tests, seems, we can make javadoc optional.
> > > > > > > > > > >
> > > > > > > > > > > What do you think?
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > > > > > > > Igniters!
> > > > > > > > > > > >
> > > > > > > > > > > > I think it's time to change coding guidelines in
> part of
> > > > > > JavaDoc
> > > > > > > > > comments
> > > > > > > > > > > > [1]:
> > > > > > > > > > > > > > Every method, field or initializer public,
> private or
> > > > > > protected
> > > > > > > > > in
> > > > > > > > > > > >
> > > > > > > > > > > > top-level,
> > > > > > > > > > > > > > inner or anonymous type should have at least
> minimal
> > > > > > Javadoc
> > > > > > > > > comments
> > > > > > > > > > > >
> > > > > > > > > > > > including
> > > > > > > > > > > > > > description and description of parameters using
> > > @param,
> > > > > > @return
> > > > > > > > > and
> > > > > > > > > > > >
> > > > > > > > > > > > @throws Javadoc tags,
> > > > > > > > > > > > > > where applicable.
> > > > > > > > > > > >
> > > > > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal
> class:
> > > > > > > > > > > >
> > > > > > > > > > > > Why?
> > > > > > > > > > > >
> > > > > > > > > > > > /** */ - 15 matches.
> > > > > > > > > > > >
> > > > > > > > > > > > What can you get new from these comments?
> > > > > > > > > > > >
> > > > > > > > > > > > /** Periodic starvation check interval. */
> > > > > > > > > > > > private static final long
> PERIODIC_STARVATION_CHECK_FREQ
> > > =
> > > > > > 1000 *
> > > > > > > > 30;
> > > > > > > > > > > >
> > > > > > > > > > > > /** Long jvm pause detector. */
> > > > > > > > > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > > > > > > > > >
> > > > > > > > > > > > /** Scheduler. */
> > > > > > > > > > > >     private IgniteScheduler scheduler;
> > > > > > > > > > > >
> > > > > > > > > > > > /** Stop guard. */
> > > > > > > > > > > >     private final AtomicBoolean stopGuard = new
> > > > > > AtomicBoolean();
> > > > > > > > > > > >
> > > > > > > > > > > > /**
> > > > > > > > > > > >      * @param cfg Configuration to use.
> > > > > > > > > > > >      * @param utilityCachePool Utility cache pool.
> > > > > > > > > > > >      * @param execSvc Executor service.
> > > > > > > > > > > >      * @param sysExecSvc System executor service.
> > > > > > > > > > > >      * @param stripedExecSvc Striped executor.
> > > > > > > > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > > > > > > > >      * @param mgmtExecSvc Management executor
> service.
> > > > > > > > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > > > > > > > >      * @param dataStreamExecSvc data stream executor
> > > service.
> > > > > > > > > > > >      * @param restExecSvc Reset executor service.
> > > > > > > > > > > >      * @param affExecSvc Affinity executor service.
> > > > > > > > > > > >      * @param idxExecSvc Indexing executor service.
> > > > > > > > > > > >      * @param callbackExecSvc Callback executor
> service.
> > > > > > > > > > > >      * @param qryExecSvc Query executor service.
> > > > > > > > > > > >      * @param schemaExecSvc Schema executor service.
> > > > > > > > > > > >      * @param customExecSvcs Custom named executors.
> > > > > > > > > > > >      * @param errHnd Error handler to use for
> > > notification
> > > > > > about
> > > > > > > > > startup
> > > > > > > > > > > > problems.
> > > > > > > > > > > >      * @param workerRegistry Worker registry.
> > > > > > > > > > > >      * @param hnd Default uncaught exception handler
> > > used by
> > > > > > thread
> > > > > > > > > pools.
> > > > > > > > > > > >      * @throws IgniteCheckedException Thrown in case
> of
> > > any
> > > > > > errors.
> > > > > > > > > > > >      */
> > > > > > > > > > > >     public void start(
> > > > > > > > > > > >         final IgniteConfiguration cfg,
> > > > > > > > > > > >         ExecutorService utilityCachePool,
> > > > > > > > > > > >         final ExecutorService execSvc,
> > > > > > > > > > > >         final ExecutorService svcExecSvc,
> > > > > > > > > > > >         final ExecutorService sysExecSvc,
> > > > > > > > > > > >         final StripedExecutor stripedExecSvc,
> > > > > > > > > > > >         ExecutorService p2pExecSvc,
> > > > > > > > > > > >         ExecutorService mgmtExecSvc,
> > > > > > > > > > > >         ExecutorService igfsExecSvc,
> > > > > > > > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > > > > > > > >         ExecutorService restExecSvc,
> > > > > > > > > > > >         ExecutorService affExecSvc,
> > > > > > > > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > > > > > > > >         IgniteStripedThreadPoolExecutor
> callbackExecSvc,
> > > > > > > > > > > >         ExecutorService qryExecSvc,
> > > > > > > > > > > >         ExecutorService schemaExecSvc,
> > > > > > > > > > > >         @Nullable final Map<String, ? extends
> > > ExecutorService>
> > > > > > > > > > > > customExecSvcs,
> > > > > > > > > > > >         GridAbsClosure errHnd,
> > > > > > > > > > > >         WorkersRegistry workerRegistry,
> > > > > > > > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > > > > > > > >         TimeBag startTimer
> > > > > > > > > > > >     )
> > > > > > > > > > > >
> > > > > > > > > > > > These comments look ugly and useless, and that is the
> > > main
> > > > > > class of
> > > > > > > > > core.
> > > > > > > > > > > > Why do they need us?
> > > > > > > > > > > > Let us change the coding guidelines in part of
> JavaDoc
> > > > > > comments:
> > > > > > > > > > > > Every method public API should have at least minimal
> > > Javadoc
> > > > > > > > > comments,
> > > > > > > > > > > > including description and description of parameters
> using
> > > > > > @param,
> > > > > > > > > @return,
> > > > > > > > > > > > and @throws Javadoc tags,
> > > > > > > > > > > > where applicable.
> > > > > > > > > > > > For internal API, JavaDoc comments should be
> optional.
> > > It's up
> > > > > > to a
> > > > > > > > > > > > contributor or reviewer.
> > > > > > > > > > > >
> > > > > > > > > > > > What are you think?
> > > > > > > > > > > >
> > > > > > > > > > > > 1.
> > > > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > >
> > >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > Best Regards, Vyacheslav D.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Best regards,
> > > > > > > > > Ivan Pavlukhin
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > > Ivan Pavlukhin
> > >
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>

[Nikolay Izhikov-2]

Hello, Anton.

> I'd like to propose to have only non-obvious params explained at
> non-public method's Javadoc.

Non-obvious for whom?
Please, remember about guys who just came into Ignite community.




В Чт, 15/08/2019 в 10:05 +0300, Anton Vinogradov пишет:

> My +1 to optional Javadoc at private methods, fields, and classes.
> The reviewer should guaranty that everything is clear.
> But, in case everything is clear without redundant Javadoc there is no need
> to have it.
>
> So, my proposal is to allow /** */ for non-public fields and methods (to
> keep readability between documented and obvious fields/methods).
> Also, I'd like to propose to have only non-obvious params explained at
> non-public method's Javadoc.
>
> On Sun, Aug 11, 2019 at 9:13 AM Павлухин Иван <[hidden email]> wrote:
>
> > Maxim,
> >
> > > I'd prefer to leave the current situation with Javadoc as it is and just
> >
> > to ask to apply the patch [1][2]. Can you? :-)
> >
> > You caught me. I left my comments for that PR
> >
> > .Pavel and all,
> >
> > > I think that API of our core internal things like PageMemory, WAL, all
> >
> > existing managers and processors should be as well documented as possible.
> >
> > No doubts here.
> >
> > > Documentation should be a result of a proper code review.
> >
> > I suppose it does not mean that documentation should be written after
> > a review. I suppose it means that we should not have a poor
> > documentation *after* a review. Overall, Pavel's last message conforms
> > well with my opinion on the subject.
> >
> > чт, 8 авг. 2019 г. в 18:34, Pavel Kovalenko <[hidden email]>:
> > >
> > > I can agree that some part of javadocs we have is useless. It relates to
> > > DTOs, getters/setters without side-effects, short self-descriptive
> >
> > methods.
> > > In an ideal world, proper modularization of architecture, leading to
> > > KISS/SOLID/DRY/etc. principles, writing self-documented code should
> >
> > result
> > > in javadocs disappearing, as they become not needed.
> > > We live in a not ideal world. We don't have good architecture and can't
> > > always lead to mentioned principles, because we need sometimes sacrifice
> > > readability for optimization, fixing a critical bug, etc.
> > > I think that API of our core internal things like PageMemory, WAL, all
> > > existing managers and processors should be as well documented as
> >
> > possible.
> > > If a developer uses some module / manager / processor without looking
> > > inside, reading the only description of public methods, it's a good sign
> > > that this part of the functionality is well documented.
> > > Internal implementation should be also clear for a developer who likes to
> > > make a change inside it. Every optimization, avoiding race-condition, not
> > > obvious thing and especially crutch must be documented as detailed as
> > > possible.
> > > Documentation should be a result of a proper code review. If a reviewer
> >
> > has
> > > questions regarding any code line it should be either refactored to make
> > > this thing obvious or well documented.
> > > If a class or method is self-documented and obvious there is no need to
> > > document it anyway.
> > > if each person takes the code review as seriously as possible,
> > > documentation and code will be better automatically.
> > > Mandatory documentation in places where it's really not needed looks
> >
> > like a
> > > burden. A developer will avoid write it properly everywhere or do it
> >
> > "just
> > > for check" and this will influence on documentation with the negative
> >
> > side.
> > > Flexible approach with mandatory / optional javadocs with good code
> >
> > review
> > > will result in readability improvement overall.
> > >
> > >
> > > чт, 8 авг. 2019 г. в 17:52, Maxim Muzafarov <[hidden email]>:
> > >
> > > > Ivan,
> > > >
> > > > It is not a problem to check Javadocs at the moment code syle checking
> > > > performed, but do we really need this? And the human-factor you
> > > > mentioned above is also related to the `self-descriptive` names. I
> > > > assume, that someone now is desiring to use single-letter variables
> > > > and single-letter class names to save space an time. We will always
> > > > have such an opinion race.
> > > >
> > > > I'd prefer to leave the current situation with Javadoc as it is and
> > > > just to ask to apply the patch [1][2]. Can you? :-)
> > > >
> > > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > > [2] https://github.com/apache/ignite/pull/6760
> > > >
> > > > On Thu, 8 Aug 2019 at 17:24, Павлухин Иван <[hidden email]>
> >
> > wrote:
> > > > >
> > > > > Maxim,
> > > > >
> > > > > My main concern here is a human factor. Humans are usually not so
> >
> > good
> > > > > in keeping documentation always in sync with a code. Examples from an
> > > > > actual PR:
> > > > > /**
> > > > > * @param nodeId The remote node id.
> > > > > * @param channel The channel to notify listeners with.
> > > > > */
> > > > > private void onChannelOpened0(UUID nodeId, GridIoMessage initMsg,
> > > > > Channel channel)
> > > > >
> > > > > First, there is a mismatch between number of parameters in javadoc
> >
> > and
> > > > > code. Second, e.g. nodeId name can be made self-descriptive rmtNodeId
> > > > > name.
> > > > >
> > > > > Mandatory javadocs do not imply good javadocs. Good javadocs do not
> > > > > imply javadocs for every method and field. For me, mandatory and good
> > > > > javadocs are like communism. Sounds quite nice in theory, but not
> > > > > feasible in practice.
> > > > >
> > > > > чт, 8 авг. 2019 г. в 16:55, Denis Garus <[hidden email]>:
> > > > > >
> > > > > > Thank you, Maxim.
> > > > > >
> > > > > > > > On what we are trying to save by making Javadoc optional?
> > > > > >
> > > > > > Space and time.
> > > > > >
> > > > > > I doubt that we need a verbal description of what private method
> >
> > make.
> > > > > > Why we need it if we just can read the code?
> > > > > >
> > > > > > Bright examples:
> > > > > >
> > > > > > /**
> > > > > > * @param helper Helper to attach to kernal context.
> > > > > > */
> > > > > > private void addHelper(Object helper) {
> > > > > >     ctx.addHelper(helper);
> > > > > > }
> > > > > >
> > > > > > /**
> > > > > > * Gets "on" or "off" string for given boolean value.
> > > > > > *
> > > > > > * @param b Boolean value to convert.
> > > > > > * @return Result string.
> > > > > > */
> > > > > > private String onOff(boolean b) {
> > > > > >     return b ? "on" : "off";
> > > > > > }
> > > > > >
> > > > > > You have to support both code of method and their JavaDoc
> >
> > description,
> > > > but
> > > > > > it doesn't get any sake.
> > > > > >
> > > > > > > > Let's just help them to read the source code.
> > > > > >
> > > > > > But at the same time, public method IgniteKernal#start doesn't
> >
> > have any
> > > > > > description except enumeration of attributes with apparent notes.
> > > > > > Maybe to give it a verbal description needs one or two pages of the
> > > >
> > > > screen.
> > > > > > Does it make sense?
> > > > > >
> > > > > > чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov <[hidden email]>:
> > > > > >
> > > > > > > Igniters,
> > > > > > >
> > > > > > > I'm -1 with making Javadoc optional (except tests).
> > > > > > >
> > > > > > > Here is my patch [1] with PR [2] which fixes all the Javadoc
> >
> > comments
> > > > > > > for the IgniteKernal class mentioned above. Please, take a look.
> >
> > The
> > > > > > > review is very appreciated.
> > > > > > >
> > > > > > > On what we are trying to save by making Javadoc optional? In my
> > > >
> > > > humble
> > > > > > > opinion - Javadoc comments are mostly concern users who debug the
> > > > > > > Ignite when they have faced with some unhandled exception or
> >
> > related
> > > > > > > to the community members with an average involvement (produces a
> >
> > few
> > > > > > > small patches during the year) but not to the experienced one.
> >
> > Let's
> > > > > > > just help them to read the source code.
> > > > > > >
> > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > > > > > [2] https://github.com/apache/ignite/pull/6760
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <[hidden email]
> > > > wrote:
> > > > > > > >
> > > > > > > > +1 for making javadoc comments optional.
> > > > > > > >
> > > > > > > > - Empty and tautological comments are kind of garbage that
> >
> > reduce
> > > > > > > > readability.
> > > > > > > > - It's better to leave the entity undocumented, than write
> > > > > > > > unexpressive/misleading comment.
> > > > > > > > - Even classes may not require javadocs, e.g. simple DTOs.
> > > > > > > >
> > > > > > > > ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]>:
> > > > > > > >
> > > > > > > > > Thx for feedback!
> > > > > > > > >
> > > > > > > > > > > we have to write proper javadoc for all production
> >
> > classes,
> > > > > > > including
> > > > > > > > > internal.
> > > > > > > > >
> > > > > > > > > Nikolay, I cannot agree with it.
> > > > > > > > >
> > > > > > > > > What should be the best comment for the next fields?
> > > > > > > > > /** */
> > > > > > > > > private static final long serialVersionUID = 0L;
> > > > > > > > > or
> > > > > > > > > /** */
> > > > > > > > > @LoggerResource
> > > > > > > > > private IgniteLogger log;
> > > > > > > > >
> > > > > > > > > There are more than 8000 lines of /** */ only at the
> >
> > ignite-core
> > > > > > > module (do
> > > > > > > > > not include tests).
> > > > > > > > >
> > > > > > > > > Any comments will be redundant and just noise. Obvious
> >
> > comment
> > > > learns
> > > > > > > > > readers skip all comments.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > > identical distance/padding/margin between fields in a
> >
> > class -
> > > > is
> > > > > > > really
> > > > > > > > > cool
> > > > > > > > >
> > > > > > > > > Vyacheslav, but we have a blank line between fields. Why is
> >
> > one
> > > > blank
> > > > > > > line
> > > > > > > > > not enough?
> > > > > > > > >
> > > > > > > > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <
> >
> > [hidden email]>:
> > > > > > > > >
> > > > > > > > > > Hi,
> > > > > > > > > >
> > > > > > > > > > Denis, thank you for starting this discussion!
> > > > > > > > > >
> > > > > > > > > > My opinion here is that having a good javadoc for every
> >
> > class
> > > > and
> > > > > > > > > > method is not feasible in the real world. I am quite
> >
> > curious
> > > > to see a
> > > > > > > > > > non-trivial project which follows it. Also, all comments
> >
> > and
> > > > javadocs
> > > > > > > > > > are prone to become misleading when code changes (human
> > > >
> > > > factor). In
> > > > > > > my
> > > > > > > > > > experience good method and variable names and clear code
> > > >
> > > > organization
> > > > > > > > > > often are more helpful than javadocs.
> > > > > > > > > >
> > > > > > > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <
> > > >
> > > > [hidden email]
> > > > > > > > :
> > > > > > > > > > >
> > > > > > > > > > > I agree that useless comments look weird in the codebase.
> > > > > > > > > > >
> > > > > > > > > > > But, identical distance/padding/margin between fields in
> >
> > a
> > > > class -
> > > > > > > is
> > > > > > > > > > > really cool, and helps read the class very fast.
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> > > > > > >
> > > > > > > [hidden email]>
> > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hello, Denis.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for starting this discussion.
> > > > > > > > > > > >
> > > > > > > > > > > > I think we have to write proper javadoc for all
> >
> > production
> > > > > > > classes,
> > > > > > > > > > including internal.
> > > > > > > > > > > > We should fix useless javadoc you provide.
> > > > > > > > > > > > We should not accept patches without good javadocs.
> > > > > > > > > > > >
> > > > > > > > > > > > As for the tests, seems, we can make javadoc optional.
> > > > > > > > > > > >
> > > > > > > > > > > > What do you think?
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > > > > > > > > Igniters!
> > > > > > > > > > > > >
> > > > > > > > > > > > > I think it's time to change coding guidelines in
> >
> > part of
> > > > > > > JavaDoc
> > > > > > > > > > comments
> > > > > > > > > > > > > [1]:
> > > > > > > > > > > > > > > Every method, field or initializer public,
> >
> > private or
> > > > > > > protected
> > > > > > > > > > in
> > > > > > > > > > > > >
> > > > > > > > > > > > > top-level,
> > > > > > > > > > > > > > > inner or anonymous type should have at least
> >
> > minimal
> > > > > > > Javadoc
> > > > > > > > > > comments
> > > > > > > > > > > > >
> > > > > > > > > > > > > including
> > > > > > > > > > > > > > > description and description of parameters using
> > > >
> > > > @param,
> > > > > > > @return
> > > > > > > > > > and
> > > > > > > > > > > > >
> > > > > > > > > > > > > @throws Javadoc tags,
> > > > > > > > > > > > > > > where applicable.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Let's look at JavaDoc comments in the IgniteKernal
> >
> > class:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Why?
> > > > > > > > > > > > >
> > > > > > > > > > > > > /** */ - 15 matches.
> > > > > > > > > > > > >
> > > > > > > > > > > > > What can you get new from these comments?
> > > > > > > > > > > > >
> > > > > > > > > > > > > /** Periodic starvation check interval. */
> > > > > > > > > > > > > private static final long
> >
> > PERIODIC_STARVATION_CHECK_FREQ
> > > > =
> > > > > > > 1000 *
> > > > > > > > > 30;
> > > > > > > > > > > > >
> > > > > > > > > > > > > /** Long jvm pause detector. */
> > > > > > > > > > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > > > > > > > > > >
> > > > > > > > > > > > > /** Scheduler. */
> > > > > > > > > > > > >     private IgniteScheduler scheduler;
> > > > > > > > > > > > >
> > > > > > > > > > > > > /** Stop guard. */
> > > > > > > > > > > > >     private final AtomicBoolean stopGuard = new
> > > > > > >
> > > > > > > AtomicBoolean();
> > > > > > > > > > > > >
> > > > > > > > > > > > > /**
> > > > > > > > > > > > >      * @param cfg Configuration to use.
> > > > > > > > > > > > >      * @param utilityCachePool Utility cache pool.
> > > > > > > > > > > > >      * @param execSvc Executor service.
> > > > > > > > > > > > >      * @param sysExecSvc System executor service.
> > > > > > > > > > > > >      * @param stripedExecSvc Striped executor.
> > > > > > > > > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > > > > > > > > >      * @param mgmtExecSvc Management executor
> >
> > service.
> > > > > > > > > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > > > > > > > > >      * @param dataStreamExecSvc data stream executor
> > > >
> > > > service.
> > > > > > > > > > > > >      * @param restExecSvc Reset executor service.
> > > > > > > > > > > > >      * @param affExecSvc Affinity executor service.
> > > > > > > > > > > > >      * @param idxExecSvc Indexing executor service.
> > > > > > > > > > > > >      * @param callbackExecSvc Callback executor
> >
> > service.
> > > > > > > > > > > > >      * @param qryExecSvc Query executor service.
> > > > > > > > > > > > >      * @param schemaExecSvc Schema executor service.
> > > > > > > > > > > > >      * @param customExecSvcs Custom named executors.
> > > > > > > > > > > > >      * @param errHnd Error handler to use for
> > > >
> > > > notification
> > > > > > > about
> > > > > > > > > > startup
> > > > > > > > > > > > > problems.
> > > > > > > > > > > > >      * @param workerRegistry Worker registry.
> > > > > > > > > > > > >      * @param hnd Default uncaught exception handler
> > > >
> > > > used by
> > > > > > > thread
> > > > > > > > > > pools.
> > > > > > > > > > > > >      * @throws IgniteCheckedException Thrown in case
> >
> > of
> > > > any
> > > > > > > errors.
> > > > > > > > > > > > >      */
> > > > > > > > > > > > >     public void start(
> > > > > > > > > > > > >         final IgniteConfiguration cfg,
> > > > > > > > > > > > >         ExecutorService utilityCachePool,
> > > > > > > > > > > > >         final ExecutorService execSvc,
> > > > > > > > > > > > >         final ExecutorService svcExecSvc,
> > > > > > > > > > > > >         final ExecutorService sysExecSvc,
> > > > > > > > > > > > >         final StripedExecutor stripedExecSvc,
> > > > > > > > > > > > >         ExecutorService p2pExecSvc,
> > > > > > > > > > > > >         ExecutorService mgmtExecSvc,
> > > > > > > > > > > > >         ExecutorService igfsExecSvc,
> > > > > > > > > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > > > > > > > > >         ExecutorService restExecSvc,
> > > > > > > > > > > > >         ExecutorService affExecSvc,
> > > > > > > > > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > > > > > > > > >         IgniteStripedThreadPoolExecutor
> >
> > callbackExecSvc,
> > > > > > > > > > > > >         ExecutorService qryExecSvc,
> > > > > > > > > > > > >         ExecutorService schemaExecSvc,
> > > > > > > > > > > > >         @Nullable final Map<String, ? extends
> > > >
> > > > ExecutorService>
> > > > > > > > > > > > > customExecSvcs,
> > > > > > > > > > > > >         GridAbsClosure errHnd,
> > > > > > > > > > > > >         WorkersRegistry workerRegistry,
> > > > > > > > > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > > > > > > > > >         TimeBag startTimer
> > > > > > > > > > > > >     )
> > > > > > > > > > > > >
> > > > > > > > > > > > > These comments look ugly and useless, and that is the
> > > >
> > > > main
> > > > > > > class of
> > > > > > > > > > core.
> > > > > > > > > > > > > Why do they need us?
> > > > > > > > > > > > > Let us change the coding guidelines in part of
> >
> > JavaDoc
> > > > > > > comments:
> > > > > > > > > > > > > Every method public API should have at least minimal
> > > >
> > > > Javadoc
> > > > > > > > > > comments,
> > > > > > > > > > > > > including description and description of parameters
> >
> > using
> > > > > > > @param,
> > > > > > > > > > @return,
> > > > > > > > > > > > > and @throws Javadoc tags,
> > > > > > > > > > > > > where applicable.
> > > > > > > > > > > > > For internal API, JavaDoc comments should be
> >
> > optional.
> > > > It's up
> > > > > > > to a
> > > > > > > > > > > > > contributor or reviewer.
> > > > > > > > > > > > >
> > > > > > > > > > > > > What are you think?
> > > > > > > > > > > > >
> > > > > > > > > > > > > 1.
> > > > > > > > > > > > >
> >
> > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > --
> > > > > > > > > > > Best Regards, Vyacheslav D.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > Best regards,
> > > > > > > > > > Ivan Pavlukhin
> > > > > > > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best regards,
> > > > > Ivan Pavlukhin
> >
> >
> >
> > --
> > Best regards,
> > Ivan Pavlukhin
> >

[Anton Vinogradov-2]

Nikolay,

Params like
* @param tx Cache transaction.
* @param val Value to set.
* @param oldVal Old value.
* @param topVer Topology version.
* @param taskName Task name.
seems to be obvious to everyone.
No reason to hove the documented.

In case you doubt it obvious, just document it.

On Thu, Aug 15, 2019 at 10:12 AM Nikolay Izhikov <[hidden email]>
wrote:

> Hello, Anton.
>
> > I'd like to propose to have only non-obvious params explained at
> > non-public method's Javadoc.
>
> Non-obvious for whom?
> Please, remember about guys who just came into Ignite community.
>
>
>
>
> В Чт, 15/08/2019 в 10:05 +0300, Anton Vinogradov пишет:
> > My +1 to optional Javadoc at private methods, fields, and classes.
> > The reviewer should guaranty that everything is clear.
> > But, in case everything is clear without redundant Javadoc there is no
> need
> > to have it.
> >
> > So, my proposal is to allow /** */ for non-public fields and methods (to
> > keep readability between documented and obvious fields/methods).
> > Also, I'd like to propose to have only non-obvious params explained at
> > non-public method's Javadoc.
> >
> > On Sun, Aug 11, 2019 at 9:13 AM Павлухин Иван <[hidden email]>
> wrote:
> >
> > > Maxim,
> > >
> > > > I'd prefer to leave the current situation with Javadoc as it is and
> just
> > >
> > > to ask to apply the patch [1][2]. Can you? :-)
> > >
> > > You caught me. I left my comments for that PR
> > >
> > > .Pavel and all,
> > >
> > > > I think that API of our core internal things like PageMemory, WAL,
> all
> > >
> > > existing managers and processors should be as well documented as
> possible.
> > >
> > > No doubts here.
> > >
> > > > Documentation should be a result of a proper code review.
> > >
> > > I suppose it does not mean that documentation should be written after
> > > a review. I suppose it means that we should not have a poor
> > > documentation *after* a review. Overall, Pavel's last message conforms
> > > well with my opinion on the subject.
> > >
> > > чт, 8 авг. 2019 г. в 18:34, Pavel Kovalenko <[hidden email]>:
> > > >
> > > > I can agree that some part of javadocs we have is useless. It
> relates to
> > > > DTOs, getters/setters without side-effects, short self-descriptive
> > >
> > > methods.
> > > > In an ideal world, proper modularization of architecture, leading to
> > > > KISS/SOLID/DRY/etc. principles, writing self-documented code should
> > >
> > > result
> > > > in javadocs disappearing, as they become not needed.
> > > > We live in a not ideal world. We don't have good architecture and
> can't
> > > > always lead to mentioned principles, because we need sometimes
> sacrifice
> > > > readability for optimization, fixing a critical bug, etc.
> > > > I think that API of our core internal things like PageMemory, WAL,
> all
> > > > existing managers and processors should be as well documented as
> > >
> > > possible.
> > > > If a developer uses some module / manager / processor without looking
> > > > inside, reading the only description of public methods, it's a good
> sign
> > > > that this part of the functionality is well documented.
> > > > Internal implementation should be also clear for a developer who
> likes to
> > > > make a change inside it. Every optimization, avoiding
> race-condition, not
> > > > obvious thing and especially crutch must be documented as detailed as
> > > > possible.
> > > > Documentation should be a result of a proper code review. If a
> reviewer
> > >
> > > has
> > > > questions regarding any code line it should be either refactored to
> make
> > > > this thing obvious or well documented.
> > > > If a class or method is self-documented and obvious there is no need
> to
> > > > document it anyway.
> > > > if each person takes the code review as seriously as possible,
> > > > documentation and code will be better automatically.
> > > > Mandatory documentation in places where it's really not needed looks
> > >
> > > like a
> > > > burden. A developer will avoid write it properly everywhere or do it
> > >
> > > "just
> > > > for check" and this will influence on documentation with the negative
> > >
> > > side.
> > > > Flexible approach with mandatory / optional javadocs with good code
> > >
> > > review
> > > > will result in readability improvement overall.
> > > >
> > > >
> > > > чт, 8 авг. 2019 г. в 17:52, Maxim Muzafarov <[hidden email]>:
> > > >
> > > > > Ivan,
> > > > >
> > > > > It is not a problem to check Javadocs at the moment code syle
> checking
> > > > > performed, but do we really need this? And the human-factor you
> > > > > mentioned above is also related to the `self-descriptive` names. I
> > > > > assume, that someone now is desiring to use single-letter variables
> > > > > and single-letter class names to save space an time. We will always
> > > > > have such an opinion race.
> > > > >
> > > > > I'd prefer to leave the current situation with Javadoc as it is and
> > > > > just to ask to apply the patch [1][2]. Can you? :-)
> > > > >
> > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > > > [2] https://github.com/apache/ignite/pull/6760
> > > > >
> > > > > On Thu, 8 Aug 2019 at 17:24, Павлухин Иван <[hidden email]>
> > >
> > > wrote:
> > > > > >
> > > > > > Maxim,
> > > > > >
> > > > > > My main concern here is a human factor. Humans are usually not so
> > >
> > > good
> > > > > > in keeping documentation always in sync with a code. Examples
> from an
> > > > > > actual PR:
> > > > > > /**
> > > > > > * @param nodeId The remote node id.
> > > > > > * @param channel The channel to notify listeners with.
> > > > > > */
> > > > > > private void onChannelOpened0(UUID nodeId, GridIoMessage initMsg,
> > > > > > Channel channel)
> > > > > >
> > > > > > First, there is a mismatch between number of parameters in
> javadoc
> > >
> > > and
> > > > > > code. Second, e.g. nodeId name can be made self-descriptive
> rmtNodeId
> > > > > > name.
> > > > > >
> > > > > > Mandatory javadocs do not imply good javadocs. Good javadocs do
> not
> > > > > > imply javadocs for every method and field. For me, mandatory and
> good
> > > > > > javadocs are like communism. Sounds quite nice in theory, but not
> > > > > > feasible in practice.
> > > > > >
> > > > > > чт, 8 авг. 2019 г. в 16:55, Denis Garus <[hidden email]>:
> > > > > > >
> > > > > > > Thank you, Maxim.
> > > > > > >
> > > > > > > > > On what we are trying to save by making Javadoc optional?
> > > > > > >
> > > > > > > Space and time.
> > > > > > >
> > > > > > > I doubt that we need a verbal description of what private
> method
> > >
> > > make.
> > > > > > > Why we need it if we just can read the code?
> > > > > > >
> > > > > > > Bright examples:
> > > > > > >
> > > > > > > /**
> > > > > > > * @param helper Helper to attach to kernal context.
> > > > > > > */
> > > > > > > private void addHelper(Object helper) {
> > > > > > >     ctx.addHelper(helper);
> > > > > > > }
> > > > > > >
> > > > > > > /**
> > > > > > > * Gets "on" or "off" string for given boolean value.
> > > > > > > *
> > > > > > > * @param b Boolean value to convert.
> > > > > > > * @return Result string.
> > > > > > > */
> > > > > > > private String onOff(boolean b) {
> > > > > > >     return b ? "on" : "off";
> > > > > > > }
> > > > > > >
> > > > > > > You have to support both code of method and their JavaDoc
> > >
> > > description,
> > > > > but
> > > > > > > it doesn't get any sake.
> > > > > > >
> > > > > > > > > Let's just help them to read the source code.
> > > > > > >
> > > > > > > But at the same time, public method IgniteKernal#start doesn't
> > >
> > > have any
> > > > > > > description except enumeration of attributes with apparent
> notes.
> > > > > > > Maybe to give it a verbal description needs one or two pages
> of the
> > > > >
> > > > > screen.
> > > > > > > Does it make sense?
> > > > > > >
> > > > > > > чт, 8 авг. 2019 г. в 15:41, Maxim Muzafarov <
> [hidden email]>:
> > > > > > >
> > > > > > > > Igniters,
> > > > > > > >
> > > > > > > > I'm -1 with making Javadoc optional (except tests).
> > > > > > > >
> > > > > > > > Here is my patch [1] with PR [2] which fixes all the Javadoc
> > >
> > > comments
> > > > > > > > for the IgniteKernal class mentioned above. Please, take a
> look.
> > >
> > > The
> > > > > > > > review is very appreciated.
> > > > > > > >
> > > > > > > > On what we are trying to save by making Javadoc optional? In
> my
> > > > >
> > > > > humble
> > > > > > > > opinion - Javadoc comments are mostly concern users who
> debug the
> > > > > > > > Ignite when they have faced with some unhandled exception or
> > >
> > > related
> > > > > > > > to the community members with an average involvement
> (produces a
> > >
> > > few
> > > > > > > > small patches during the year) but not to the experienced
> one.
> > >
> > > Let's
> > > > > > > > just help them to read the source code.
> > > > > > > >
> > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12051
> > > > > > > > [2] https://github.com/apache/ignite/pull/6760
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > On Wed, 7 Aug 2019 at 13:54, Andrey Kuznetsov <
> [hidden email]
> > > > > wrote:
> > > > > > > > >
> > > > > > > > > +1 for making javadoc comments optional.
> > > > > > > > >
> > > > > > > > > - Empty and tautological comments are kind of garbage that
> > >
> > > reduce
> > > > > > > > > readability.
> > > > > > > > > - It's better to leave the entity undocumented, than write
> > > > > > > > > unexpressive/misleading comment.
> > > > > > > > > - Even classes may not require javadocs, e.g. simple DTOs.
> > > > > > > > >
> > > > > > > > > ср, 7 авг. 2019 г., 13:39 Denis Garus <[hidden email]
> >:
> > > > > > > > >
> > > > > > > > > > Thx for feedback!
> > > > > > > > > >
> > > > > > > > > > > > we have to write proper javadoc for all production
> > >
> > > classes,
> > > > > > > > including
> > > > > > > > > > internal.
> > > > > > > > > >
> > > > > > > > > > Nikolay, I cannot agree with it.
> > > > > > > > > >
> > > > > > > > > > What should be the best comment for the next fields?
> > > > > > > > > > /** */
> > > > > > > > > > private static final long serialVersionUID = 0L;
> > > > > > > > > > or
> > > > > > > > > > /** */
> > > > > > > > > > @LoggerResource
> > > > > > > > > > private IgniteLogger log;
> > > > > > > > > >
> > > > > > > > > > There are more than 8000 lines of /** */ only at the
> > >
> > > ignite-core
> > > > > > > > module (do
> > > > > > > > > > not include tests).
> > > > > > > > > >
> > > > > > > > > > Any comments will be redundant and just noise. Obvious
> > >
> > > comment
> > > > > learns
> > > > > > > > > > readers skip all comments.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > > identical distance/padding/margin between fields in a
> > >
> > > class -
> > > > > is
> > > > > > > > really
> > > > > > > > > > cool
> > > > > > > > > >
> > > > > > > > > > Vyacheslav, but we have a blank line between fields. Why
> is
> > >
> > > one
> > > > > blank
> > > > > > > > line
> > > > > > > > > > not enough?
> > > > > > > > > >
> > > > > > > > > > ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <
> > >
> > > [hidden email]>:
> > > > > > > > > >
> > > > > > > > > > > Hi,
> > > > > > > > > > >
> > > > > > > > > > > Denis, thank you for starting this discussion!
> > > > > > > > > > >
> > > > > > > > > > > My opinion here is that having a good javadoc for every
> > >
> > > class
> > > > > and
> > > > > > > > > > > method is not feasible in the real world. I am quite
> > >
> > > curious
> > > > > to see a
> > > > > > > > > > > non-trivial project which follows it. Also, all
> comments
> > >
> > > and
> > > > > javadocs
> > > > > > > > > > > are prone to become misleading when code changes (human
> > > > >
> > > > > factor). In
> > > > > > > > my
> > > > > > > > > > > experience good method and variable names and clear
> code
> > > > >
> > > > > organization
> > > > > > > > > > > often are more helpful than javadocs.
> > > > > > > > > > >
> > > > > > > > > > > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <
> > > > >
> > > > > [hidden email]
> > > > > > > > > :
> > > > > > > > > > > >
> > > > > > > > > > > > I agree that useless comments look weird in the
> codebase.
> > > > > > > > > > > >
> > > > > > > > > > > > But, identical distance/padding/margin between
> fields in
> > >
> > > a
> > > > > class -
> > > > > > > > is
> > > > > > > > > > > > really cool, and helps read the class very fast.
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <
> > > > > > > >
> > > > > > > > [hidden email]>
> > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hello, Denis.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks for starting this discussion.
> > > > > > > > > > > > >
> > > > > > > > > > > > > I think we have to write proper javadoc for all
> > >
> > > production
> > > > > > > > classes,
> > > > > > > > > > > including internal.
> > > > > > > > > > > > > We should fix useless javadoc you provide.
> > > > > > > > > > > > > We should not accept patches without good javadocs.
> > > > > > > > > > > > >
> > > > > > > > > > > > > As for the tests, seems, we can make javadoc
> optional.
> > > > > > > > > > > > >
> > > > > > > > > > > > > What do you think?
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > > > > > > > > > > Igniters!
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I think it's time to change coding guidelines in
> > >
> > > part of
> > > > > > > > JavaDoc
> > > > > > > > > > > comments
> > > > > > > > > > > > > > [1]:
> > > > > > > > > > > > > > > > Every method, field or initializer public,
> > >
> > > private or
> > > > > > > > protected
> > > > > > > > > > > in
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > top-level,
> > > > > > > > > > > > > > > > inner or anonymous type should have at least
> > >
> > > minimal
> > > > > > > > Javadoc
> > > > > > > > > > > comments
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > including
> > > > > > > > > > > > > > > > description and description of parameters
> using
> > > > >
> > > > > @param,
> > > > > > > > @return
> > > > > > > > > > > and
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > @throws Javadoc tags,
> > > > > > > > > > > > > > > > where applicable.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Let's look at JavaDoc comments in the
> IgniteKernal
> > >
> > > class:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Why?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > /** */ - 15 matches.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > What can you get new from these comments?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > /** Periodic starvation check interval. */
> > > > > > > > > > > > > > private static final long
> > >
> > > PERIODIC_STARVATION_CHECK_FREQ
> > > > > =
> > > > > > > > 1000 *
> > > > > > > > > > 30;
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > /** Long jvm pause detector. */
> > > > > > > > > > > > > > private LongJVMPauseDetector
> longJVMPauseDetector;
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > /** Scheduler. */
> > > > > > > > > > > > > >     private IgniteScheduler scheduler;
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > /** Stop guard. */
> > > > > > > > > > > > > >     private final AtomicBoolean stopGuard = new
> > > > > > > >
> > > > > > > > AtomicBoolean();
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > /**
> > > > > > > > > > > > > >      * @param cfg Configuration to use.
> > > > > > > > > > > > > >      * @param utilityCachePool Utility cache
> pool.
> > > > > > > > > > > > > >      * @param execSvc Executor service.
> > > > > > > > > > > > > >      * @param sysExecSvc System executor service.
> > > > > > > > > > > > > >      * @param stripedExecSvc Striped executor.
> > > > > > > > > > > > > >      * @param p2pExecSvc P2P executor service.
> > > > > > > > > > > > > >      * @param mgmtExecSvc Management executor
> > >
> > > service.
> > > > > > > > > > > > > >      * @param igfsExecSvc IGFS executor service.
> > > > > > > > > > > > > >      * @param dataStreamExecSvc data stream
> executor
> > > > >
> > > > > service.
> > > > > > > > > > > > > >      * @param restExecSvc Reset executor service.
> > > > > > > > > > > > > >      * @param affExecSvc Affinity executor
> service.
> > > > > > > > > > > > > >      * @param idxExecSvc Indexing executor
> service.
> > > > > > > > > > > > > >      * @param callbackExecSvc Callback executor
> > >
> > > service.
> > > > > > > > > > > > > >      * @param qryExecSvc Query executor service.
> > > > > > > > > > > > > >      * @param schemaExecSvc Schema executor
> service.
> > > > > > > > > > > > > >      * @param customExecSvcs Custom named
> executors.
> > > > > > > > > > > > > >      * @param errHnd Error handler to use for
> > > > >
> > > > > notification
> > > > > > > > about
> > > > > > > > > > > startup
> > > > > > > > > > > > > > problems.
> > > > > > > > > > > > > >      * @param workerRegistry Worker registry.
> > > > > > > > > > > > > >      * @param hnd Default uncaught exception
> handler
> > > > >
> > > > > used by
> > > > > > > > thread
> > > > > > > > > > > pools.
> > > > > > > > > > > > > >      * @throws IgniteCheckedException Thrown in
> case
> > >
> > > of
> > > > > any
> > > > > > > > errors.
> > > > > > > > > > > > > >      */
> > > > > > > > > > > > > >     public void start(
> > > > > > > > > > > > > >         final IgniteConfiguration cfg,
> > > > > > > > > > > > > >         ExecutorService utilityCachePool,
> > > > > > > > > > > > > >         final ExecutorService execSvc,
> > > > > > > > > > > > > >         final ExecutorService svcExecSvc,
> > > > > > > > > > > > > >         final ExecutorService sysExecSvc,
> > > > > > > > > > > > > >         final StripedExecutor stripedExecSvc,
> > > > > > > > > > > > > >         ExecutorService p2pExecSvc,
> > > > > > > > > > > > > >         ExecutorService mgmtExecSvc,
> > > > > > > > > > > > > >         ExecutorService igfsExecSvc,
> > > > > > > > > > > > > >         StripedExecutor dataStreamExecSvc,
> > > > > > > > > > > > > >         ExecutorService restExecSvc,
> > > > > > > > > > > > > >         ExecutorService affExecSvc,
> > > > > > > > > > > > > >         @Nullable ExecutorService idxExecSvc,
> > > > > > > > > > > > > >         IgniteStripedThreadPoolExecutor
> > >
> > > callbackExecSvc,
> > > > > > > > > > > > > >         ExecutorService qryExecSvc,
> > > > > > > > > > > > > >         ExecutorService schemaExecSvc,
> > > > > > > > > > > > > >         @Nullable final Map<String, ? extends
> > > > >
> > > > > ExecutorService>
> > > > > > > > > > > > > > customExecSvcs,
> > > > > > > > > > > > > >         GridAbsClosure errHnd,
> > > > > > > > > > > > > >         WorkersRegistry workerRegistry,
> > > > > > > > > > > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > > > > > > > > > > >         TimeBag startTimer
> > > > > > > > > > > > > >     )
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > These comments look ugly and useless, and that
> is the
> > > > >
> > > > > main
> > > > > > > > class of
> > > > > > > > > > > core.
> > > > > > > > > > > > > > Why do they need us?
> > > > > > > > > > > > > > Let us change the coding guidelines in part of
> > >
> > > JavaDoc
> > > > > > > > comments:
> > > > > > > > > > > > > > Every method public API should have at least
> minimal
> > > > >
> > > > > Javadoc
> > > > > > > > > > > comments,
> > > > > > > > > > > > > > including description and description of
> parameters
> > >
> > > using
> > > > > > > > @param,
> > > > > > > > > > > @return,
> > > > > > > > > > > > > > and @throws Javadoc tags,
> > > > > > > > > > > > > > where applicable.
> > > > > > > > > > > > > > For internal API, JavaDoc comments should be
> > >
> > > optional.
> > > > > It's up
> > > > > > > > to a
> > > > > > > > > > > > > > contributor or reviewer.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > What are you think?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > 1.
> > > > > > > > > > > > > >
> > >
> > >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > --
> > > > > > > > > > > > Best Regards, Vyacheslav D.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > --
> > > > > > > > > > > Best regards,
> > > > > > > > > > > Ivan Pavlukhin
> > > > > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best regards,
> > > > > > Ivan Pavlukhin
> > >
> > >
> > >
> > > --
> > > Best regards,
> > > Ivan Pavlukhin
> > >
>