Page MenuHomeSoftware Heritage

Add javadoc documentation in graph service
ClosedPublic

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

Details

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

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.

haltode marked 14 inline comments as done.

Fix grammar/style issues.

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

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)

This revision is now accepted and ready to land.Jul 17 2019, 3:23 PM
This revision was automatically updated to reflect the committed changes.