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 
<stku...@gmail.com> 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 <garus....@gmail.com>:

> 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, Павлухин Иван <vololo...@gmail.com>:
>
> > 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 <daradu...@gmail.com>:
> > >
> > > 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 <nizhi...@apache.org>
> > 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
> >
>  

Reply via email to