Moti Asayag has posted comments on this change.

Change subject: core: errors in javadoc, badly placed method.
......................................................................


Patch Set 2: Code-Review+1

Thanks for referring to the review comments.

> a) when I step into class, where A LOT of javadocs are broken. Should I 
> correct them (in separate commit like this one)? 

This is up to the developer to decide whether he wishes to improve an existing 
code/javadoc or not. Assuming you have a limited of time to invest in such, you 
can prioritize on what is most important to you/project.

> Is it observed as beneficial to project / welcomed by other developers? 

It is most welcome. Regarding transperancy to other developers - i read 
methods' javadoc which aid to understand a complex logic.

> b) is there a convention for javadoc? For example I'm very liberal about 
> them. 

I'm not aware of such convention

> If they're not faulty, they're good enough for me as long as the method 
> responsibility/purpose is clear; which includes no javadoc. Lets say that I 
> observe documentation of parameter 'taskId' of method 'create', which says 
> "id of task to create", as useless. Is it OK to "not document" such 
> parameters or if javadoc is present all parameters has to be documented 
> somehow? How about present empty parameter documentation? I'm used to 
> dropping that, because it does not offer nothing more that method signature 
> already provides.

I agree trivial code doesn't require documentation, but omitting attributes 
might result someday in incomplete generated javadoc (if there will be someday)

Specifically regarding the taskId, we can provide more information to the user 
about its constrains (i.e. could not be null) so users will not think that 
passing null will generate a new id. But the main reason is to differentiate 
this field from AsyncTaskCreationInfo.vdsmTaskID which is another task id being 
provided at the same creation and requires to be distinguished from the prior 
one (taskId).

-- 
To view, visit http://gerrit.ovirt.org/26065
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I334d7080989d679fa27082547f1a5950fb530645
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <[email protected]>
Gerrit-Reviewer: Martin Mucha <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to