Don't panic

[kevburnsjr]

Patch should never panic.

2019/12/27 21:38:19 template: history.html:23:18: executing "history.html" at <.Name>: error calling Name: Invalid path element destinations.

The method has an opportunity to return an error.
There is no reason to panic.

I do not want to have to defer/recover just to guard against some diff library that sees fit to halt execution of my entire program.

[josephburnett]

You're right. This lib should never panic.

20 looks good. We should also remove the panics here:

https://github.com/josephburnett/jd/blob/dac2309936f8cb8fbca851051a00d4c6409cdecd/lib/diff.go#L23 https://github.com/josephburnett/jd/blob/dac2309936f8cb8fbca851051a00d4c6409cdecd/lib/diff.go#L34

[kevburnsjr]

The only reason I left those is because they're in the Render() call stack and Render can't return an error.

[josephburnett]

Yeah, I wonder if Render should return an error. There isn't a way to create a JsonNode with this library that won't serialize. So I could ignore the error return value. However the JsonNode interface is public so it is possible to have an error.

[josephburnett]

If I was to change the signature of Render to return an error, would it break you? The diff and diff element types didn't have an interface until recently.

[kevburnsjr]

I think if you changed the signature you'd probably break a lot of peoples' code :man_shrugging:

Render() string is certainly convenient. I imagine a lot of people just toss it into a one-liner.

[josephburnett]

For a one-liner and putting into debugging strings, I think String() would be a better signature. I haven't made any breaking changes yet. But Render() should probably return an error. I might collect a few more things to change, adopt go modules, then bump to 2.0.

[josephburnett]

you'd probably break a lot of peoples' code

I have no idea how widely used this library is. Curious, how are you using it?

[kevburnsjr]

One rough metric to determine how many people are using a project is Traffic Insights https://github.com/josephburnett/jd/graphs/traffic

Only you can see this.

That will at least tell you how many clones you get per day.

If you see a large number of clones and a small number of views, it's probably safe to assume there is a build system somewhere that is cloning your repo repeatedly for every build.

Example from another project:

I was going to use jd to serve as an audit trail for document change history. Every time a document gets updated, the diff gets stored in a separate time ordered table. This would allow the object to be rebuilt from history and could enable undo functionality (in theory).

Again though, I decided not to use jd. If I'm going to build multiple features around a json object history, I'd prefer to use RFC 6902.

Not only is it supported in many languages but the diffs themselves are json which makes them much easier to work with. Extracting those Name and ID fields from the diff is much easier when the diff itself is json.

[
  {
    "op": "replace",
    "path": "/sources/s7stu3gcs0iaybhu16tyv777t3scezvy/name",
    "value": "Javascript"
  }
]

I've come to believe that jsonpatch is the industry standard for solving this class of problems.