tgdwyer / WebCola

Javascript constraint-based graph layout
http://marvl.infotech.monash.edu/webcola/
MIT License
2.02k stars 258 forks source link

routeEdge(): sudden API changes, broken examples, outdated docs #255

Open codethief opened 6 years ago

codethief commented 6 years ago

First of all, let me say that WebCola is a great library and I am very happy with how great it works! Thanks for all the work you're putting in!

Regarding the issue: I'm trying to get routeEdge() to work (in TypeScript, not JavaScript) and I am absolutely flabbergasted when it comes to the differences in call signature between

  1. the current code here on GitHub which lists 3 mandatory parameters for routeEdge() (albeit ah comes with a default value but this doesn't really matter because it is followed by a non-optional parameter, making ah itself non-optional, see the TypeScript docs)
  2. the online docs which list two mandatory parameters for routeEdge()
  3. the online example which passes precisely one argument to routeEdge()
  4. my local version of WebCola (3.3.6) which expects two arguments for routeEdge()

To give you an impression of just how confusing this is, here's what I did: I mostly copied the code from the online example (3), i.e. just passed one argument to routeEdge() only to then be greeted by a TypeScript error (see 4): "Expected 2 arguments, but got 1". So I checked the online docs (see 2) and had no clue what the draw parameter is supposed to be as it's not documented. (I hadn't copied the commented-out draw argument from the online example and had forgotten it was there.) I figured that there might have been an update to WebCola in the meantime, explaining why the online example only passes one argument. So I updated to WebCola 3.3.8. The result: "Expected 3 arguments, but got 1." What gives?! Only then I checked WebCola's source code in detail and realized:

  1. that there here had been a recent breaking change of routeEdge()'s call signature, which is neither reflected in the online example nor in the docs.
  2. what the purpose of draw is and that it is entirely optional (routeEdge() is checking whether it's undefined) but isn't declared as such.

I would therefore suggest:

  1. to document the draw argument, i.e. that it is can be used to draw the visibility graph
  2. to update the online example (I know it's using JavaScript, so we're allowed to pass fewer arguments to routeEdge(), but violating the TypeScript API is really confusing)
  3. to update the online docs to reflect the API changes (I would have expected that it's automatically generated from the repository's code?)
  4. not to introduce breaking changes to the API anymore in future minor version changes (like 3.3.6 => 3.3.8). Things like this make it really hard to use WebCola in big software projects where stability of the software and its dependencies is key.

My initial comment notwithstanding (I am really happy with the results I'm getting with WebCola!), I would like to mention that this is not the first time I find it a bit tiring that WebCola is not properly documented. I regularly find out about features, function parameters and so on only because I'm browsing code examples and GitHub issues. Meanwhile, the online API reference right now is not really helpful. I was therefore about to create a meta issue like "Document all WebCola functions and every single one of their parameters" but wanted to ask for your opinion first. What do you guys think?

codethief commented 6 years ago

Oops, I just noticed that there had been a huge copy & paste error in my bug report. Fixed it! :)

gordonwoodhull commented 6 years ago

Are you volunteering to write those docs? :wink:

codethief commented 6 years ago

Well, the problem with that would be that I don't really understand a lot of parameters. I've just largely been fiddling around with some of them (without a clear understanding of what exactly they do) until the results looked fine. So I don't think I'd be the right person here.

Besides, this is also a question of (your) development habits – documenting is something that, at least in my opinion, should be done along the way, i.e. while coding or at least before releasing a new version. Put differently, what purpose would my documentation serve if you won't keep it up-to-date? ;)

gordonwoodhull commented 5 years ago

My apologies to @codethief and @tgdwyer. It wasn't appropriate for me to make such a comment on someone else's repo.

I get frustrated by the entitled attitude of users of open source, but I can see that @codethief was trying to make a positive contribution here.

codethief commented 5 years ago

@gordonwoodhull: Oops, I just realized that, when I responded to your post above, for some reason I must have been mistaking you for @tgdwyer as I was talking about "you" as the developer behind WebCola. Sorry about that!

As for your most recent post: Absolutely no offense taken! I totally understand where you were coming from!