On Wed, Oct 26, 2011 at 11:05 AM, Nick Sieger <nicksie...@gmail.com> wrote:
> Looks good for the most part. Two things I'd like to see:
>
> 1. Put the descriptions in javadocs for each field. API docs are going
> to be more popular for browsing the full set of configuration options.
> Does the description/toString ever get used in a meaningful way?

That's not a bad thought, though I'm not sure how best to get those
descriptions into --properties. The description you see there is
automatically being composed into our command-line --properties
output. That was one goal of this...to have property metadata and
description all in one place, so we could generate doco and handle
property loading all from the same structure.

> 2. Do away with the static-ness of the fields. Can we get away with
> them being instance variables and a Properties-instance-per-runtime?
> I'd rather see that approach with a fallback to a statically loaded
> value. That would allow different runtimes in the same JVM to be
> configured differently, either through updating system properties or
> via an API that gives access to the Properties object (that was the
> original intent for RubyInstanceConfig).

The Properties object may be a bit misleading. It doesn't actually
have any state. The Property objects are just metadata + load logic
for the various config properties, and the values they load are stored
elsewhere...some are instance fields on RubyInstanceConfig and some
are still statics, but I didn't change anything on that end of things
yet. This commit *only* centralizes the load logic and metadata.

> One final mention -- naming the class Properties clashes with
> java.util.Properties too easily for me.

Yeah, I agree. Trying to think of a better way to describe it.
PropertyMetadata or something? MetaProperties? It's not really a
"Properties" object at all, since it has no state.

- Charlie

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply via email to