JVM object encode implementation

[bzz]

Addresses #90

This is based on #91 and thus includes it history and need to be rebased as soon as it's merged.
The only new part is 6eedc24

It is a pre-request for further work on iterate()/filter() API in #83 and thus needs to be merged first, before continuing the work.

It includes:

• Context/ContextExt separation for JVM/Go managed UASTs, same as in python client
• ability to create Context on JVM side and encode it

---

This change is 

[dennwc]

Reviewable status: 0 of 20 files reviewed, 1 unresolved discussion (waiting on @bzz, @creachadair, and @dennwc)

a discussion (no related file):
This will have to wait for a rebase. I believe there are changes that overlap with recent changes to #91.

---

[bzz]

Reviewable status: 0 of 20 files reviewed, 1 unresolved discussion (waiting on @creachadair and @dennwc)

a discussion (no related file):
👍 but of course there are, that's what PR description explains :)

This is based on #91 and thus includes it history and need to be rebased as soon as it's merged.
The only new part is 6eedc24

---

[dennwc]

Reviewable status: 0 of 20 files reviewed, 1 unresolved discussion (waiting on @bzz and @creachadair)

a discussion (no related file):

Previously, bzz (Alexander) wrote…

👍 but of course there are, that's what PR description explains :)

This is based on #91 and thus includes it history and need to be rebased as soon as it's merged.
The only new part is 6eedc24

Right, but I got an impression that some (new) changes overlap with 6eedc24.
And I can't effectively review it without a real diff comparing it to a new state of #91 specifically, sorry.

---

[bzz]

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @creachadair and @dennwc)

a discussion (no related file):

Previously, dennwc (Denys Smirnov) wrote…

Right, but I got an impression that some (new) changes overlap with 6eedc24.
And I can't effectively review it without a real diff comparing it to a new state of #91 specifically, sorry.

Rebased!

Oh, did not realize that - viewing single commit in github UI worked fine and rebase did not have any conflicts

But should be good to go now, sorry for confusion!

---

[bzz]

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @creachadair and @dennwc)

a discussion (no related file):

Previously, bzz (Alexander) wrote…

Rebased!

Oh, did not realize that - viewing single commit in github UI worked fine and rebase did not have any conflicts

But should be good to go now, sorry for confusion!

Done.

---

[dennwc]

Reviewed 11 of 20 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @bzz and @creachadair)

a discussion (no related file):

Previously, bzz (Alexander) wrote…

Done.

That's much better :) Thanks!

---

---

src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc, line 106 at r2 (raw file):

  // Borrows the reference.
  jobject Encode(jobject node, UastFormat format) {
    // if (!assertNotContext(node)) return nullptr;

Any TODO comment for this one?

---

src/main/scala/org/bblfsh/client/v2/ContextExt.scala, line 6 at r2 (raw file):

/**
  * pyuast.ContextExt

pyuast? Is it intentional?

---

src/test/scala/org/bblfsh/client/v2/BblfshClientLoadTest.scala, line 72 at r2 (raw file):

    data should equal (resp.uast.asReadOnlyByteBuffer())
//    data.compareTo(resp.uast.asReadOnlyByteBuffer()) shouldBe 0

Remove the comment, since the check above works?

[bzz]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @creachadair and @dennwc)

---

src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc, line 106 at r2 (raw file):

Previously, dennwc (Denys Smirnov) wrote…

Any TODO comment for this one?

🤦‍♂️ simply forgot, let me add this now and fix == nullptr

---

src/main/scala/org/bblfsh/client/v2/ContextExt.scala, line 6 at r2 (raw file):

Previously, dennwc (Denys Smirnov) wrote…

pyuast? Is it intentional?

Yes, it is. Let me try to phrase it better - I want to emphasize/document the API parity between python and Scala.

Would

Represents Go-side results of Libuast.decode()

This is equivalent of pyuast.ContextExt API

be better?

---

src/test/scala/org/bblfsh/client/v2/BblfshClientLoadTest.scala, line 72 at r2 (raw file):

Previously, dennwc (Denys Smirnov) wrote…

Remove the comment, since the check above works?

yup, thanks! The above actually does include the same byte-to-byte comparison so 👍

[dennwc]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @bzz, @creachadair, and @dennwc)

---

src/main/scala/org/bblfsh/client/v2/ContextExt.scala, line 6 at r2 (raw file):

Previously, bzz (Alexander) wrote…

Yes, it is. Let me try to phrase it better - I want to emphasize/document the API parity between python and Scala.

Would

Represents Go-side results of Libuast.decode()

This is equivalent of pyuast.ContextExt API

be better?

Yes, I think that comment will be more descriptive.

As a side note, can you please also switch the state of comment to "Working" or "Blocked" on your end for relevant comments?

[bzz]

Sorry for delay, got caught up in my language class today.

Pushed the changes, ready for another round @dennwc (still can not belive reviewable does not auto-complete GH handles!)

Reviewable status: 8 of 11 files reviewed, 3 unresolved discussions (waiting on @bzz, @creachadair, and @dennwc)

---

src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc, line 106 at r2 (raw file):

Previously, bzz (Alexander) wrote…

🤦‍♂️ simply forgot, let me add this now and fix == nullptr

Done.

---

src/main/scala/org/bblfsh/client/v2/ContextExt.scala, line 6 at r2 (raw file):

As a side note, can you please also switch the state of comment to "Working" or "Blocked" on your end for relevant comments?

Thanks for the pro-tip! I just did not realize it's the thing to do 🤦‍♂️

Done.

---

src/test/scala/org/bblfsh/client/v2/BblfshClientLoadTest.scala, line 72 at r2 (raw file):

Previously, bzz (Alexander) wrote…

yup, thanks! The above actually does include the same byte-to-byte comparison so 👍

Done

[bzz]

Reviewable status: 8 of 11 files reviewed, 3 unresolved discussions (waiting on @creachadair and @dennwc)

---

src/main/scala/org/bblfsh/client/v2/ContextExt.scala, line 6 at r2 (raw file):

Previously, bzz (Alexander) wrote…

As a side note, can you please also switch the state of comment to "Working" or "Blocked" on your end for relevant comments?

Thanks for the pro-tip! I just did not realize it's the thing to do 🤦‍♂️

Done.

Done.

[bzz]

CI seems to be failing - I'll investigate and post back

[bzz]

Done, @dennwc, ready for another round.

Reviewable status: 8 of 11 files reviewed, 3 unresolved discussions (waiting on @creachadair and @dennwc)

[dennwc]

Reviewed 3 of 3 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @creachadair)

[bzz]

I'm going to go ahead and merge this, in order to avoid the complication of branching the further work off this PR, given that we want to release a SNAPSHOT early next week for a demo.