Add javadoc documentation in graph service
ClosedPublic
Actions

Authored by haltode on Jul 16 2019, 3:47 PM.

Details

Reviewers

zack
seirl

Commits

rDGRPHfc9dd235c28b: server: add javadoc documentation

Summary

First pass to add javadoc in the server side of swh-graph.

What needs to be done still:

Figure out how to link to the swh-graph/docs
Fix external documentation links (eg: to webgraph framework)
Add more technical details (eg: Graph, mapping files, etc.)

First step towards T1904.

Diff Detail

Repository

rDGRPH Compressed graph representation

Branch

javadoc

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 6887
Build 9653: arc lint + arc unit

Event Timeline

haltode created this revision.Jul 16 2019, 3:47 PM

Harbormaster completed remote builds in B6887: Diff 5851.Jul 16 2019, 3:47 PM

haltode edited the summary of this revision. (Show Details)Jul 16 2019, 3:48 PM

in addition to the comments below, a general issue is that all verbs are in the third person (e.g., "returns foo", "converts bla"), while it is more idiomatic to use the infinitive in API documentation (e.g., "return foo", "convert bla").

api/server/src/main/java/org/softwareheritage/graph/AllowedEdges.java
69	This reads weird. suggested alternative: "Checks if a given edge can be followed during graph traversal."
api/server/src/main/java/org/softwareheritage/graph/Endpoint.java
66	"using" is weird here. Rather: "Converts a list of path made of (internal) long node IDs to one made of SwhPath-s."
81	you can drop "function" from here
95	drop "function"
109	drop "function"
139	drop "function"
153	drop "function"
api/server/src/main/java/org/softwareheritage/graph/Graph.java
32	As we have many different kinds of "path" in the code, this deserves a qualifier, e.g.: "full on-disk path of the graph". Also, is it really an actual path to a single file, or rather a basename to which different extensions will be attached? Either way, this should be updated to clarify.
34–37	when describing mappings it's generally useful to be explicit about the direction, e.g.: mapping from internal long IDs to external SWH PIDs mapping from internal long IDs to node types or even: mapping: internal long IDs -> external SWH PIDs mapping: internal long IDs -> node types (and you can probably drop "internal" too, as it's redundant)
42	as above, please clarify what this path points too, as we've plenty of related files/dirs
69	can be shortened: "converts {@link SwhId} node to long"
80	can be shortened: "converts long node it to {@link SwhId}"
api/server/src/main/java/org/softwareheritage/graph/Node.java
56	"parses SWH node type from string"
67	"Parse ... from string"

zack requested changes to this revision.Jul 17 2019, 2:51 PM

This revision now requires changes to proceed.Jul 17 2019, 2:51 PM

In D1741#40365, @zack wrote:

in addition to the comments below, a general issue is that all verbs are in the third person (e.g., "returns foo", "converts bla"), while it is more idiomatic to use the infinitive in API documentation (e.g., "return foo", "convert bla").

I was following Oracle convention from the Javadoc page: https://www.oracle.com/technetwork/articles/javase/index-137868.html

"Use 3rd person (descriptive) not 2nd person (prescriptive)."

But I can change it if we want to be more consistent with SWH overall documentation.

Fix grammar/style issues.

Harbormaster completed remote builds in B6899: Diff 5863.Jul 17 2019, 3:12 PM

Concerning links to swh-graph/docs it needs to wait for integration with swh-docs and then add external html links.

In D1741#40411, @haltode wrote:

Concerning links to swh-graph/docs it needs to wait for integration with swh-docs and then add external html links.

it's fine to integrate it now as is, and fix dangling links later (they're marked with easy-to-find TODO markers anyway)

zack accepted this revision.Jul 17 2019, 3:23 PM

This revision is now accepted and ready to land.Jul 17 2019, 3:23 PM

haltode edited the summary of this revision. (Show Details)Jul 17 2019, 3:27 PM

Rebasing on master.

Harbormaster completed remote builds in B6902: Diff 5866.Jul 17 2019, 4:12 PM

Closed by commit rDGRPHfc9dd235c28b: server: add javadoc documentation (authored by haltode). · Explain WhyJul 17 2019, 4:12 PM

This revision was automatically updated to reflect the committed changes.