Rfc geoscript console #164

Merged
merged 50 commits into from Jan 16, 2013

Conversation

Projects
None yet
2 participants
Contributor

moovida commented Jan 13, 2013

This is the pull request to get the Geoscript Console in uDig.

It also contains the doc page for the editor.

RFC here: http://udig.refractions.net/confluence/display/UDIG/Addition+of+a+Geoscript+Console

moovida added some commits Jan 3, 2013

Contributor

jodygarnett commented on 8a38933 Jan 15, 2013

Thanks Moovida! It really helps having an example

@@ -13,7 +13,18 @@ bin.includes = .,\
about.html,\
@jodygarnett

jodygarnett Jan 15, 2013

Contributor

I see about.html mentioned, but cannot figure out how to view it from github. So with about.html, ell and hsd3 license we should be good to go

@moovida

moovida Jan 16, 2013

Contributor

The about.html has not been edited. It is probably mentioned in the diff, because it has been moved when updating the libs?

@@ -13,7 +13,18 @@ bin.includes = .,\
about.html,\
bsd3-v10.html,\
@jodygarnett

jodygarnett Jan 15, 2013

Contributor

This may be the hydrologis take on a BSD license (often hsd3-v10.html)

@moovida

moovida Jan 16, 2013

Contributor

What do you mean here?

@jodygarnett

jodygarnett Jan 16, 2013

Contributor

I was looking for they three key license files (about.html summary, epl license, and hsd license)

Contributor

jodygarnett commented Jan 15, 2013

I found it easiest to browse https://github.com/uDig/udig-platform/tree/rfc_geoscript_console/plugins/eu.udig.jconsole to review the proposed files.

License review:

  • epl and hsd check out okay (yay!)
  • about.html is missing (this is what the help dialog shows, it links to esd and hsd files)

Docs:

  • we have documentation page!

Spot check on headers:

  • JConsolePlugin <-- missing header
  • JavaActionContributor.java <-- IBM header (assume they did not set this up for reuse, will need to note in about.html)
  • JavaEditorDocumentProvider.java <-- standard hydroloGIS header (EPL and HSD)

Suggestions for discussion:

  • So we should search for missing headers? Either now or do an IP check later (need to find an example of about.html)
  • The JavaEditorDocumentProvider, this is from an example I presume, since we are not editing in Java can this be refactored to say GeoScriptDocumentProvider?

Suggestion: Accept this pull request as is (so it does not go stale) and add a few tasks to the RFC:

  • IP check of headers and about.html
  • Refactor for GeoScript rather than Java (or even GSDocumentProvider
Contributor

moovida commented Jan 16, 2013

Comments inline.

License review:

  • epl and hsd check out okay (yay!)
  • about.html is missing (this is what the help dialog shows, it links to esd and hsd files)

What is this thing about the about.html file? What do you want me to do?

Spot check on headers:

  • JConsolePlugin <-- missing header

outch, adding it after pull if ok.

  • JavaActionContributor.java <-- IBM header (assume they did not set this up for reuse, will need to note in about.html)

Why do you think it is not for reuse. Isn't it EPL, which is our base license? It should be ok?

  • JavaEditorDocumentProvider.java <-- standard hydroloGIS header (EPL and HSD)

ok

Suggestions for discussion:

  • So we should search for missing headers? Either now or do an IP check later (need to find an example of about.html)
  • The JavaEditorDocumentProvider, this is from an example I presume, since we are not editing in Java can this be refactored to say GeoScriptDocumentProvider?

Yes, you are right, All the Java* should get Geoscript*. I liked the fact that it would support also Groovy, so I wa snot shure what to rename it. So I forgot :)

Suggestion: Accept this pull request as is (so it does not go stale) and add a few tasks to the RFC:

  • IP check of headers and about.html
  • Refactor for GeoScript rather than Java (or even GSDocumentProvider

Agreed on everything. Need more directions regarding the about file.

Contributor

jodygarnett commented Jan 16, 2013

More information on about.html:

Or uDig specific:

jodygarnett added a commit that referenced this pull request Jan 16, 2013

Merge pull request #164 from uDig/rfc_geoscript_console
Initial merge of geoscript console review resulted in some tasks for the RFC

@jodygarnett jodygarnett merged commit b0e786f into master Jan 16, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment