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

A few Codo bugs #64

Closed
aseemk opened this issue Sep 6, 2012 · 15 comments
Closed

A few Codo bugs #64

aseemk opened this issue Sep 6, 2012 · 15 comments
Labels

Comments

@aseemk
Copy link

aseemk commented Sep 6, 2012

I've gone through one of our public projects and added Codo-style documentation to each file:

https://github.com/thingdom/node-neo4j/tree/feature/documentation

Unfortunately, the generated documentation is close but not quite there:

https://dl.dropbox.com/u/25149872/node-neo4j/doc/index.html

I hope you don't mind if I share the various bugs here in one issue instead of opening up a bunch of separate issues. =)

Take a look at the GraphDatabase class for instance; source:

https://github.com/thingdom/node-neo4j/blob/feature/documentation/lib/GraphDatabase._coffee

It appears that the ### Database: ### comment on line 39 messes up all the documentation for the first few methods. (And it also gets treated as the description for the first method.)

Somehow, the ### Nodes: ### comment on line 118 doesn't cause this problem, and things return to normal for those node methods. But the problem reappears for the ### Relationships: ### comment on line 249.

Things again return to being documented with the query() method, but the overloads' documentation somehow gets nested instead of shown one after the other. And then the next method's documentation also gets nested within the overloads.

Finally, for the PropertyContainer and Relationship classes, none of the methods have any generated documentation; source:

https://github.com/thingdom/node-neo4j/blob/feature/documentation/lib/PropertyContainer._coffee
https://github.com/thingdom/node-neo4j/blob/feature/documentation/lib/Relationship._coffee

You can see that I'm using the strawman @property tag I proposed in #61. The GraphDatabase and Node classes don't use this and so don't have this problem. Here, they seem to be breaking everything.

So this is where I'm at. Quite close! I'd love to know if I'm doing anything wrong, but FYI on these seeming bugs otherwise. Excited to get this working. Thanks again. =)

P.S. Btw, I first tried coffeedoc.info to share the documentation, but it doesn't quite work nearly at all:

http://coffeedoc.info/github/thingdom/node-neo4j/feature/documentation/

Not only do none of the classes have almost any generated documentation, the Path class is missed altogether. It's alos using the latest Codo 1.3.1 (though on Node 0.8), so I'm not sure what's up.

@netzpirat
Copy link
Contributor

Thanks a lot for the detailed information. I really appreciate it and I'll takle down each of these issues next week, since I'm a bit short on time for this week.

@netzpirat
Copy link
Contributor

See the commit message for more info on the block comment issue. The other issue with missing comments is, that properties aren't implemented yet :P

@aseemk
Copy link
Author

aseemk commented Sep 7, 2012

Awesome!

Any idea why coffeedoc.info still doesn't seem to work? (Alternately, is there any way to regenerate the documentation for an existing commit? I just retyped the branch name in but maybe that doesn't have any effect.)

http://coffeedoc.info/github/thingdom/node-neo4j/feature/documentation/

@aseemk
Copy link
Author

aseemk commented Sep 7, 2012

Sorry, ignore my question: I realize this update hasn't even been released yet.

@netzpirat
Copy link
Contributor

You can always have a look at http://queue.coffeedoc.info/ to see job status and failures. I'll release a new Codo version soon.

@aseemk
Copy link
Author

aseemk commented Sep 7, 2012

Awesome, had no idea of that site. Thanks for the great work, and no rush at all.

netzpirat added a commit that referenced this issue Sep 7, 2012
@netzpirat
Copy link
Contributor

Codo 1.4.0 is out with all the fixes!

@aseemk
Copy link
Author

aseemk commented Sep 7, 2012

Awesome! I will test it out later and report back. Thanks so much for all the fantastic work and timeliness!

@aseemk
Copy link
Author

aseemk commented Sep 10, 2012

Cool, so this definitely improves things a bunch, but there are still a few issues FYI.

Btw, CoffeeDoc.info's version still doesn't really work (I regenerated):

http://coffeedoc.info/github/thingdom/node-neo4j/feature/documentation/

So here's the documentation I generated locally again:

https://dl.dropbox.com/u/25149872/node-neo4j/doc/index.html

First, an HTTP link in the GraphDatabase::query() description isn't getting recognized. Warning in console:

$ codo --title 'Node-Neo4j API Documentation' ./lib

{ reference: '<a',
  label: 'href="http://docs.neo4j.org/chunked/stable/cypher-query-lang.html">http://docs.neo4j.org/chunked/stable/cypher-query-lang.html</a>' }
[WARN] Cannot find referenced class <a in class GraphDatabase
{ reference: '<a',
  label: 'href="http://docs.neo4j.org/chunked/stable/cypher-query-lang.html">http://docs.neo4j.org/chunked/stable/cypher-query-lang.html</a>' }
[WARN] Cannot find referenced class <a in class GraphDatabase
Parsed files:           7
Classes:                5 (0 undocumented)
Mixins:                 0
Non-Class files:        6
Methods:               34 (0 undocumented)
Constants:              0 (0 undocumented)
 100.00% documented

Source comment:

https://github.com/thingdom/node-neo4j/blob/feature/documentation/lib/GraphDatabase._coffee#L370

Second, the GraphDatabase::queryNodeIndex() method isn't getting documented/included by Codo. This could very well be because there is a piece of "executable class body" code right before it:

https://github.com/thingdom/node-neo4j/blob/feature/documentation/lib/GraphDatabase._coffee#L487

Next, there are a few spots where text is somehow getting broken up -- very strange:

  • GraphDatabase constructor. The 'http://user:password@example.com/' string should be part of the @param url; it's instead getting randomly included as part of the description.
  • GraphDatabase::getIndexedNode(). The "See #getIndexedNodes for returning multiple nodes" should be part of the @note; it's instead getting cut off.
  • Same with GraphDatabase::getIndexedRelationship().

But I think I know what the issue is — the commonality with all of these is that in the source code, these three have those code or link texts beginning on lines of their own. Codo seems to be missing those?

Finally, this may be the same bug as the previous, but the PropertyContainer::exists property has its description mangled a bit. The description source has two lines:

https://github.com/thingdom/node-neo4j/blob/feature/documentation/lib/PropertyContainer._coffee#L45-L49

But Codo is showing the second line of text before the first line, weird enough.

Sorry to keep on "bugging" you =), but hope this helps! Let me know if I can provide more info. Thanks again!

@netzpirat
Copy link
Contributor

I've also seen the issue with the re-generation of the docs on coffeedoc.info. I think this is a wrong Mongo query for projects with a branch other than master. I've opened an issue coffeedoc/coffeedoc.info#1 and will look at the problem.

@netzpirat
Copy link
Contributor

I've also opened #69 for the failing link parsing (this should work and is a bug), #70 for the missing method and #71 for the property description.

I really love your feedback, it will make Codo better for all of us. I only prefer to have the different issues separated. It makes them easier to manage.

@aseemk
Copy link
Author

aseemk commented Sep 11, 2012

Awesome, thanks for being so open to this kind of feedback. Got it re: multiple issues -- I wasn't sure if that would be annoying to have a flood of new issues open up. Thanks!

@netzpirat
Copy link
Contributor

You're welcome. I'll try to resolve all these issues this week, but I must first get some real (payed) work done before I can reward myself with some nice OSS work :P

@aseemk
Copy link
Author

aseemk commented Sep 11, 2012

Always understandable; I need to get back to my real work too. =D

@aseemk
Copy link
Author

aseemk commented Sep 30, 2012

Thanks for taking care of all of these bugs! I also was "distracted" by real work ;) but returning to this, it's been great to see that so much has been fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants