Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated code documentation #20

Merged
merged 7 commits into from Dec 8, 2014
Merged

Updated code documentation #20

merged 7 commits into from Dec 8, 2014

Conversation

snaterlicious
Copy link
Contributor

Allows generating a proper documentation using JSDuck.
(A custom JSDuck tag class for "licence" tag should be applied upon generation.)

@@ -16,18 +19,21 @@ var SELF = wb.datamodel.Entity = function WbDataModelEntity() {

/**
* String to identify this type of Entity.
* @type {string}
* @type {null}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's odd. In the subclasses this is a string. I think this must be {string|null}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure about that. It is never set via a constructor and stands for itself here but it probably makes more sense to keep it on {string}.

@thiemowmde
Copy link
Contributor

I read everything and left a few comments. Looking forward to merge this. Good work.

@@ -113,7 +122,8 @@ wb.datamodel.EntityId = util.inherit(
} );

/**
* @see dataValues.DataValue.newFromJSON
* @inheritdoc
* @static
*/
wb.datamodel.EntityId.newFromJSON = function( json ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing param/return documentation. (wb.datamodel.EntityId.TYPE below needs documentation as well)

@snaterlicious snaterlicious force-pushed the documentation branch 2 times, most recently from 1ccd2dc to d8a3026 Compare November 25, 2014 10:59
@thiemowmde
Copy link
Contributor

I don't agree with dropping the @see. This is one of the very few very commonly used tags in all out doc blocks. A limitation in a specific doc parser should not force us to limit our vocabulary in code docs. However, no reason to block this patch. I will merge it but it needs to be rebased, it seems.

thiemowmde added a commit that referenced this pull request Dec 8, 2014
Updated code documentation
@thiemowmde thiemowmde merged commit 9049dbf into master Dec 8, 2014
@thiemowmde thiemowmde deleted the documentation branch December 8, 2014 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants