commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rahul Akolkar (JIRA)" <>
Subject [jira] Updated: (SCXML-84) [PATCH] SCXML on Android Usecase
Date Wed, 01 Oct 2008 22:24:44 GMT


Rahul Akolkar updated SCXML-84:

    Fix Version/s: 0.9

This looks good, I have some suggestions below -- can you please make these changes and post
a fresh patch and zip:

Suggested changes to the patch file:

 * All new files need the Apache license header (just like the one you added in scxml-stopwatch-on-android.xml).
Obviously, XML files should use XML comments for the header, Java files Java-style comments
and so on. For diff.txt, you can use any comment format you want above the first line that
shows the command-line invocation :-)
 * A note about attribution: When the patch is applied with the changes suggested here, your
name will be added to the team page on the web site (that is how we credit contributions to
the project): , but I think having the sample
in a seemingly unrelated package ("") would be potentially confusing
to our users/readers. Can you replace that with ""
in this patch?

Minor comments about the patch file:

 * As a general comment about style of the usecase document, I think it will be better if
it isn't written in first person (the entire document isn't written this way, just the two
sections "Handling unmet dependencies" and "Building the .apk" -- I think it will be good
to be consistent). So, as a concrete example, this text -- "After investigating the source
code of the dependencies I could get a clearer picture of dependencies to J2SE. Based on what
I've concluded I created an UML Package Diagram." -- could be replaced by this instead --
"Here is a UML Package Diagram that draws out the picture of dependencies to J2SE after investigating
the source code of the missing dependencies." or some such sentence that doesn't use the first
 * Will need to place as on the site so
browsers like IE can actually view the file without much fuss.

Minor comments on the scxml-dependencies-on-android.jpg image:

 * I know the Maven 2 generated dependencies page says that, but Commons SCXML does not actually
require Commons Lang. It should be removed from that picture.
 * Can we have a color coding for optional dependencies? The javax dependencies are all optional,
it would be good to mark them as such (even JEXL and EL are optional in the sense that you
would use only one expression language at a time).
 * Typo: org.apache.commons.el dependency has JEXL heading <<commons-jexl>>, should
be <<commons-el>>
 * Typo in comment at bottom: s/Neighter/Neither/

Thanks in advance.

> [PATCH]  SCXML on Android Usecase
> ---------------------------------
>                 Key: SCXML-84
>                 URL:
>             Project: Commons SCXML
>          Issue Type: New Feature
>         Environment: Android, Android SDK, Eclipse ADT
>            Reporter: Jakob Sachse
>             Fix For: 0.9
>         Attachments:, patch.txt
> please find attached the patch

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message