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