search

researchart / rose7re20

3 stars 1 forks source link

Reviews on submission 174 #11

Open fabianodalp opened 4 years ago

fabianodalp commented 4 years ago

The assigned reviewers are going to post their reviews on this submission within this issue. The same thread will be used also to support the interaction with the authors.

Reviewers, please check STATUS.md to determine which badges the artifact is applying for. A description of the badges can be found here: https://re20.org/index.php/artifacts/. You will also receive an e-mail with further instructions shortly.

michvier commented 4 years ago

Badges applied for: Reusable, Available

Available Badge:

The data is available on zenodo with the doi and link being provided.

Reusable badge:

I found the installation instructions, especially for the IDE rather hard to follow and complicated. For someone who is not familiar with XText and/or Eclipse plug-in development, installing and getting the plug-in up and running might be quite challenging.

As an easy way to get everything up and running, I would recommend to (in addition to the source code) maybe provide an update site where one can install your plug without going through all the installation steps?

As for the Installation Instructions:

- Symboleo-IDE:

Providing these 2 different ways seems to be overly complicated (the first one did not work for me).

Eclipse 2019-09R or later. Xtext v2.19.0 or later.

For someone not familiar with Xtext who wants to use the IDE, I would recommend to point them to a guide on how to install the XText/Xtend Framework in eclipse (e.g., provide a link to the update site) or guide them directly to the Eclipse Package that already contains Xtext ( Eclipse for DSL developers - https://www.eclipse.org/downloads/packages/release/2020-03/r/eclipse-ide-java-and-dsl-developers)

I think all the projects, not only the org.xtext.csmLab.symboleo, need to be imported? (org.xtext.csmLab.symboleo.ide, .ui, ...) otherwise you get compile errors. Also, the src-gen folder was not in the classpath after importing the project (had to be added manually to get rid of some of the errors in the manifest file)

However, even with this working and getting rid of the compile errors the plug-in did not work properly and caused exceptions when running it.

Open org.xtext.csmLab.symboleo/src/org/xtext/csmLab/symboleo/Symboleo.xtext from the Github repo in notepad and replace it with the contents of the same file in the eclipse xtext project.

I was a bit confused by these instructions on what to exactly replace with what (and why).

Issues aside with the first way, the 2nd way described in the installation instructions however did work.

Symboleo Compliance Checker

Again there were some minor issues with the installation instructions:

When trying to import and run the tool in eclipse there are a few issues: There are dependencies required that are not part of a standard Eclipse installation:

Maybe if you could provide this as a maven project (with dependencies being resolved automatically) it would be easier to get it running in the IDE

Again, the second way, using the linked jar file did work.

Using the artifact

As for the reusability:

... very carefully documented and well-structured to the extent that reuse and repurposing is facilitated.

It would be really helpful if you could, besides the installation instructions, provide a readme briefly describing the usage for both, with a link to the sample files. A quick tutorial on how to use the editor and the compliance checker (e.g., on how to write contracts) This is partially already there for the ide (but part of the installation instructions)

dfucci commented 4 years ago

Badges applied for Available and Reusable

Available

The artifact is accessible from a publically-accessible archival repository (Zenodo). A DOI is present in the submission ✅

Reusable

Tested on MacOS Catalina (10.15.5) with Eclipse DSL Tools Version: 2020-03 (4.15.0)

Symboleo-IDE

  1. I agree with @michvier that the instructions need (and can be easily) improved.
  2. I was able to run using the 2nd approach (Building an xtext project in Eclipse) ✅
  3. Synthax highlight works ✅
  4. The auto-completion seems to be working only for some elements (e.g., type E followed by Control+Space completes Enumeration but not Event). Previously-declared symbols (e.g., buyer in the example) are not autocompleted. Not sure this is the expected behavior❓

Symboleo-compliance-checker

  1. The file names used as input for the ComplianceChecker constructor in the App class are not portable (e.g., use \ separator, which breaks the execution on *nix machines) ❌

  2. Symboleo.jar displays the expected output but the process does not terminate (e.g., I need to kill it manually with control+C). Not sure this is the expected behavior since it hinders the construction of pipes based on the output❓

fabianodalp commented 4 years ago

Thank you @dfucci and @michvier for youre reviews. @SepShr can you please respond to them? I have assigned the "available" badge already.

SepShr commented 4 years ago

Thank you @michvier and @dfucci for your very constructive comments. It will help us improve the artifacts considerably.

Regarding the question that @dfucci asked in comment No.4: What you experienced is the expected behavior. The current version of the IDE provides auto-completion for many keywords in the contract but has limited auto-completion for the previously-defined symbols (if they are defined in the domain section, the auto-complete would not recognize and suggest them in the contract section and vice versa). In the case of suggesting Enumeration but not Event, this is also expected as the former is defined directly in the grammar rule, while the ontological and basic types (such as Event) are defined in a nested-rule fashion. The result would be the IDE not suggesting Event but highlighting it when written in the code.

Regarding the @dfucci 's question in comment No.5: we are working on the project to fix portability of constructors and dependency.

Regarding the @dfucci 's question in comment No.6: This result is expected as the program is supposed to execute all traces. However, we will modify the program to either terminate on its own or notify the user to terminate.

Regarding @michvier 's point on improving the artifacts for reusability: We appreciate the comments and implement the changes in IDE's installation. Additionally, we will improve the Readme.md file for both projects.

SepShr commented 4 years ago

@fabianodalp , @michvier and @dfucci ; we have implemented suggestions: both README files are improved and now contain short tutorials on how to use the artifact.

We appreciate your re-evaluation for the reusable badge.

dfucci commented 4 years ago

Hello @SepShr, thanks for the effort. Can you point me to the commit that fixes my comment #5? I still see the previous implementation here

SepShr commented 4 years ago

@michvier and @dfucci I apologize for the delay. We have created a made the project available in a Maven build to fix the portability and dependencies issues.

Please let me know if the issue still persists.

dfucci commented 4 years ago

@SepShr great! The Maven build works as expected. Minor: the instruction on how to run it are Windows-specific.

michvier commented 4 years ago

@SepShr Thanks for the changes. Regarding the compliance checker part: importing everything in Eclipse & running the maven build is now working nicely (without the need to install additional libraries). Works on Linux as well.

The installation instructions for the eclipse plug-in are much clearer and simpler now as well.

fabianodalp commented 4 years ago

Excellent! Thank you @dfucci and @michvier . Both badges are awarded. Thank you @SepShr for the fixes, now your code is more open science friendly :)

MonaRahimi commented 4 years ago

@SepShr thank you for the changes. For the camera ready please make sure you update your instructions (if necessary) to also be easy to follow in Linux-based environments. Thank you.

SepShr commented 4 years ago

Thank you @dfucci , @michvier , @fabianodalp and @MonaRahimi for the comments that has helped us improve the usability work considerably. On behalf of the team, I would like to thank you for your time and effort for implementing this very valuable evaluation/improvement process.