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

JsDoc fixes for issue #11 #80

Merged
merged 14 commits into from
Jun 28, 2012
Merged

JsDoc fixes for issue #11 #80

merged 14 commits into from
Jun 28, 2012

Conversation

kristiancalhoun
Copy link
Contributor

Reintroduce @exports tag to document modules.
Switch from @name to @alias to document object properties.
Added some @defaults to CentralBody.
@enumerations no longer list 'new' in their documentation as though they were a class.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 26, 2012

A few comments/questions:

  • The doc is still missing all the GLSL functions. Is that because they are in .glsl files? Should that be fixed with this pull request?
  • On the main doc page, should switching between all, js, and glsl clear the filter?
  • Did we switch to var foo = function... to workaround a jsdoc-toolkit bug? If so, did we tell them?
  • I find the use of both @exports and @alias verbose, but if that is how it has to be, it is OK with me.

@kristiancalhoun
Copy link
Contributor Author

The doc is still missing all the GLSL functions. Is that because they are in .glsl files? Should that be fixed with this pull request?

Yes, it is. I tried changing the JsDoc configuration file to read .glsl files as well as .js files, but JsDoc chokes on the different syntax. @shunter, do you have any ideas? Is stripping out the comments during the build process and writing them to another file for JsDoc to read unreasonable?

On the main doc page, should switching between all, js, and glsl clear the filter?
Yup, that would definitely improve usability.

Did we switch to var foo = function... to workaround a jsdoc-toolkit bug? If so, did we tell them?

Not so much a bug as the design of JsDoc. The problem is that function foo() is within an anonymous function inside of the module definition, so JsDoc won't document it. We previously got around this problem by using the @name tag, which basically tells JsDoc to just look at the comment and not the code. However, then documentation for the inner properties is not generated (using '@constructor foo' has the same problem). var foo = function is the recommended way to document classes within modules as seen here : http://usejsdoc.org/howto-commonjs-modules.html.

If you're interested in reading more: https://groups.google.com/forum/?fromgroups#!searchin/jsdoc-users/function$20Sample/jsdoc-users/L1n50YvtwFY/aB2gVbxD4RwJ

I find the use of both @exports and @alias verbose, but if that is how it has to be, it is OK with me.
You're right. I'll cut out the @exports where @alias is present.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 28, 2012

@kristiancalhoun let us know when this is ready for review again.

@kristiancalhoun
Copy link
Contributor Author

It's good to go.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 28, 2012

I still don't see any GLSL functions. Do I need to clean build?

@kristiancalhoun
Copy link
Contributor Author

Yeah, try that. I just did another clean build and it worked for me.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 28, 2012

Can you please tweak a few minor things:

@kristiancalhoun
Copy link
Contributor Author

agi_emptyRaySegment is duplicated in BuiltinFunctions.glsl. It looks to be a copy/paste error, as one should be agi_fullRaySegment.

@agi_complement, @agi_insertAt, and @agi_intersection are duplicated because they're overloaded functions and each is tagged.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 28, 2012

OK. Can you fix them? Should be trivial.

@kristiancalhoun
Copy link
Contributor Author

Alright, 5055141 takes care of those issues.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 28, 2012

agi_complement, agi_insertAt, and agi_intersection all return an HTTP error.

@kristiancalhoun
Copy link
Contributor Author

Interseting. They all work for me from both the index page and sidebar. Did you try another clean build? What is the broken URL for each?

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 28, 2012

Yes, I cleaned. It is probably because I am using the filter: http://localhost:8080/Build/Documentation/global.html?classFilter=agi_com#agi_complement.

@kristiancalhoun
Copy link
Contributor Author

global.html is purposefully not generated (because nothing should link there), which is why you're getting an error. I searched through my entire documentation folder and couldn't find anything that linked to global.html. Regardless, sync up and try again with this latest commit.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 28, 2012

Looks good now.

@mramato I suppose you'll want to pull master in to get this.

pjcozzi added a commit that referenced this pull request Jun 28, 2012
@pjcozzi pjcozzi merged commit b3795a5 into master Jun 28, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants