[REVIEW]: Golo - a lightweight dynamic language for the JVM

[whedon]

Submitting author: @jponge (Julien Ponge)
Repository: https://github.com/eclipse/golo-lang
Version: v3.2.0
Editor: @danielskatz
Reviewer: @stevenrbrandt
Archive: 10.5281/zenodo.192097

Status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/e39c59c4d49bfa1cdd3d753cca253157"><img src="http://joss.theoj.org/papers/e39c59c4d49bfa1cdd3d753cca253157/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/e39c59c4d49bfa1cdd3d753cca253157/status.svg)](http://joss.theoj.org/papers/e39c59c4d49bfa1cdd3d753cca253157)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer questions

Conflict of interest

• As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

General checks

• Repository: Is the source code for this software available at the repository url?
• License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• Version: Does the release version given match the GitHub release (v3.2.0)?
• Authorship: Has the submitting author (@jponge) made major contributions to the software?

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• Performance: Have any performance claims of the software been confirmed?

Documentation

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g. API method documentation)?
• Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

Paper PDF: 10.21105.joss.00093.pdf

• Authors: Does the paper.md file include a list of authors with their affiliations?
• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• References: Do all archival references that should have a DOI list one (e.g. papers, datasets, software)?

[whedon]

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS.

For a list of things I can do to help you, just type:

@whedon commands

[danielskatz]

Thanks @stevenrbrandt for agreeing to do this - the review is this issue.

The reviewer guidelines are here: http://joss.theoj.org/about#reviewer_guidelines

For the most part you need to see if the statements above in the first post are true and check things off accordingly. If something isn't the case, or you otherwise have a suggestion, just post a comment to @jponge here (or ask me) and we'll sort things out until the submission meets all of JOSS' guidelines.

[jponge]

I've made some updates, see @arfon's feedback eclipse-archived/golo-lang#410

[whedon]

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the reviewer of this submission
@whedon assign @username as reviewer

# List the GitHub usernames of the JOSS editors
@whedon list editors

# List of JOSS reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Open the review issue
@whedon start review

🚧 Important 🚧

This is all quite new. Please make sure you check the top of the issue after running a @whedon command (you might also need to refresh the page to see the issue update).

[danielskatz]

@stevenrbrandt , you shouldn't have to use whedon as a reviewer. you can just check boxes, and make comments here (in the issue) for the author to respond to.

[stevenrbrandt]

I'm having difficulty with the statement of need. All I see is this: "The world didn't need another JVM language. So we built yet another one. A simple one." I see a better statement of need here (http://www.javamagazine.mozaicreader.com/MarApr2016/Twitter#&pageSet=54&page=0), but I think it ought to go somewhere on the website or in the documentation.

[danielskatz]

@stevenrbrandt - Is this all of your feedback for the authors to respond to? Or should they wait for more feedback?

[stevenrbrandt]

@danielskatz So far, I haven't found any other problems. I'm still kicking the tires. What's the deadline?

[danielskatz]

A goal is to have the reviewer's first comments in a week or so after the review starts. Maybe you can finish by the end of this week?

[stevenrbrandt]

Sure.

[stevenrbrandt]

OK, here's a new issue. My command-line tests worked, but I am unable to get the Eclipse plugin to function. Not sure if that's my fault somehow. I filed an issue at golo-lang/gldt#4

[jponge]

@stevenrbrandt the Eclipse plugin is (ironically) unmaintained at this stage and is not part of the submission.

[stevenrbrandt]

One of the purposes of golo is to teach people how to implement a dynamic runtime language (as stated in the paper). Given that, it seems to me there should be some document introducing the overall structure of the source code, maybe some tutorial on how to add a language feature. I don't see these things as blocking issues, however, just suggestions.

[stevenrbrandt]

@jponge, given that the Eclipse plugin isn't maintained and isn't working, maybe it should be taken off the list of editors.

[stevenrbrandt]

shebang doesn't work for me. All the other methods for compiling and running this same source file do work
$ ./build/install/golo/bin/golo shebang hello-1.golo [error] In Golo module: duplicated-struct.golo [error] Declaring a typeFootwice (declared first here: {line=7, column=1}) at {line=9, column=1}

[stevenrbrandt]

@jponge if you can help me with the shebang, I can close out this review.

[jponge]

@stevenrbrandt we are not removing the reference to the Eclipse IDE plugin, it keeps contributions hopes alive.

Regarding the shebang issue it seems to me that you are running duplicated-struct.golo from our tests suite and get an error. This is fine, this file is used by the test suite to check that an error is actually being detected at compilation time.

[stevenrbrandt]

@jponge How do I prevent the running of the duplicate struct-golo? I'm not explicitly doing that.

[jponge]

Could you please share the code snippet that raises the error, and if it comes from the Golo source code point me to the file path?

I do suspect that you are running a file that is actually meant to raise an error as part of our unit tests.

[stevenrbrandt]

I copied it from the tutorial.

module javamag.Hello

import java.util

# A comment
function main = |args| {
  let elements = ArrayList()
  elements: add("Hello")
  elements: add("World")
  elements: add("!")
  let size = elements:size()
  for(var i=0, i < size, i = i + 1) {
    print(elements:get(i))
    if(i < size - 1) {
      print(" ")
    }
  }
  println("")
}

[jponge]

$ pbpaste 
module javamag.Hello

import java.util

# A comment
function main = |args| {
  let elements = ArrayList()
  elements: add("Hello")
  elements: add("World")
  elements: add("!")
  let size = elements:size()
  for(var i=0, i < size, i = i + 1) {
    print(elements:get(i))
    if(i < size - 1) {
      print(" ")
    }
  }
  println("")
}

$ pbpaste > hello.golo
$ golo golo --files hello.golo 
Hello World !
$

Am I missing anything?

[stevenrbrandt]

@jponge Nope. I don't get that result. Not sure what's different about my installation.
I do these things:
git clone https://github.com/eclipse/golo-lang
./gradlew build
./gradlew test
./gradlew clean
./gradlew installDist
./build/install/golo/bin/golo shebang hello-1.golo

And I get that error message I sent to you.

In case it's useful
$ uname -a
Linux localhost.localdomain 4.4.14-200.fc22.x86_64 #1 SMP Fri Jun 24 21:19:33 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
$ java -version
java version "1.8.0_65"
Java(TM) SE Runtime Environment (build 1.8.0_65-b17)
Java HotSpot(TM) 64-Bit Server VM (build 25.65-b01, mixed mode)

[jponge]

I'm on a fresher version of the JVM:

$ java -version
java version "1.8.0_112"
Java(TM) SE Runtime Environment (build 1.8.0_112-b16)
Java HotSpot(TM) 64-Bit Server VM (build 25.112-b16, mixed mode)

I did the same steps, here's the transcript.

$ git clone https://github.com/eclipse/golo-lang
Cloning into 'golo-lang'...
remote: Counting objects: 24291, done.
remote: Compressing objects: 100% (12/12), done.
remote: Total 24291 (delta 4), reused 0 (delta 0), pack-reused 24279
Receiving objects: 100% (24291/24291), 5.12 MiB | 2.47 MiB/s, done.
Resolving deltas: 100% (9428/9428), done.
Checking connectivity... done.
fuji:Code jponge$ cd golo-lang/

Then I did a ./gradlew build installDist, which builds just fine.

I then went with check.golo as follows:

module javamag.Hello

import java.util

# A comment
function main = |args| {
  let elements = ArrayList()
  elements: add("Hello")
  elements: add("World")
  elements: add("!")
  let size = elements:size()
  for(var i=0, i < size, i = i + 1) {
    print(elements:get(i))
    if(i < size - 1) {
      print(" ")
    }
  }
  println("")
}

It runs fine:

$ build/install/golo/bin/golo golo --files check.golo 
Hello World !

[stevenrbrandt]

@jponge , that's not the shebang command. The files command works for me.

$ build/install/golo/bin/golo golo --files hello-1.golo
Hello World !
$ build/install/golo/bin/golo shebang hello-1.golo
[error] In Golo module: duplicated-struct.golo
[error] Declaring a type Foo twice (declared first here: {line=7, column=1}) at {line=9, column=1}

[jponge]

Ok I think I understand now :-) shebang is special as it scans for all .golo and .jar files starting from your current working directory. You should try to move the file and you current working directory somewhere else then. Does it work for you?

[stevenrbrandt]

@jponge, Yes, putting it in its own directory does make a difference. That point is mentioned in the docs, but I didn't recognize that my problem might be caused by that feature. Maybe you could elaborate on this feature in the docs to make it clearer why golo scans the subdirectories and what functionality that enables. Also, you might mention the problems it might create in the pitfalls section.

[jponge]

Good point, I'll think about adding this in the documentation.

[jponge]

Hi, Any update regarding this review / what are the next steps? Thanks!

[danielskatz]

I think we're waiting for you to tell us you've made the changes suggested by @stevenrbrandt in the documentation.

[danielskatz]

@stevenrbrandt - it looks like the two unchecked boxes are
Functionality: Have the functional claims of the software been confirmed?
Performance: Have any performance claims of the software been confirmed?

Will the documentation fix that you suggested allow you to check the functionality box?

If the software doesn't make performance claims, this box can be checked.
If it does make performance claims, then those need to be reviewed before the box can be checked.

[jponge]

@danielskatz I've just pushed documentation changes to address @stevenrbrandt feedback, see eclipse-archived/golo-lang@6167b71

[jponge]

Ping?

[stevenrbrandt]

I had thought I'd commented that it looked good--not sure where the comment went.

[danielskatz]

Ok, thanks - I see that everything is now checked.

@arfon, this one is ready to accept! :)

[arfon]

@arfon, this one is ready to accept! :)

Excellent. Thanks @danielskatz.

@jponge - at this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

[jponge]

@arfon here you go! https://zenodo.org/record/192097

[arfon]

@whedon set 10.5281/zenodo.192097 as archive

[whedon]

OK. 10.5281/zenodo.192097 is the archive.

[arfon]

@stevenrbrandt many thanks for the review. @danielskatz thanks for editing this submission.

@jponge your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00093 🚀 🎉 💥

[jponge]

Thanks everyone :-)

…

On Fri, Dec 2, 2016 at 1:52 AM, Arfon Smith ***@***.***> wrote: Closed #93 <#93>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#93 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABlaVlEDc0IO6QB8sK9zi17IkxKWGIAks5rD2vRgaJpZM4KXFXB> .