[GitHub] [geode] kirklund commented on issue #2915: GEODE-6117: Makes modules out of geode-core and geode-cq

2018-12-06 Thread GitHub
Thumbs up! Already approved.

[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org


[GitHub] [geode] kirklund commented on issue #2915: GEODE-6117: Makes modules out of geode-core and geode-cq

2018-12-06 Thread GitHub
The difference between `org.apache.geode.cache.query` and 
`org.apache.geode.query` is fairly small but they are non-internal User APIs. 
So for fixing modularization I would lean away from changing the User packages 
-- or do so as little as possible. The impression I got on the dev-list was 
that we were just shuffling around internal packages without reshaping our 
non-internal User packages. You should probably be more explicit about exposing 
or altering User packages (especially on the dev-list). That's also why I was 
asking for `org.apache.geode.cache.query.cq` instead of `org.apache.geode.cq` 
-- because I feel like we're making a pretty permanent decision that impacts 
the User API/package for CQ. It'll be next to impossible to change that after 
Geode 1.9.0 releases.

[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org


[GitHub] [geode] kirklund commented on issue #2915: GEODE-6117: Makes modules out of geode-core and geode-cq

2018-12-06 Thread GitHub
I forgot geode-cq is a module but geode-query is not [edited]. It still seems 
like cq is part of query(ing) though. We can't use 
org.apache.geode.cache.query.cq inside geode-cq? Or is that just convention? 
Would we ever create geode-query to contain org.apache.geode.cache.query and 
then geode-cq adds cq onto that?

I'm thinking that geode-core will eventually contain org.apache.geode.logging 
to contain our LogService and SPI for the LogWriter and Alert appenders (or 
even move that to geode-logging). Then geode-log4j would contain our default 
impls of that SPI which are the LogWriter and Alert appenders that are written 
with log4j-core. Then geode-core uses geode-logging and geode-log4j is an 
optional provider to the SPI in geode-logging. Or is that too many sub-modules 
(I'm new to modules!)?

So, my input on cq is based mainly on cq being something that builds on query. 
If org.apache.geode.cache.query.cq is ugly because of geode-cq or if we would 
never split out query to geode-query then my suggestion probably isn't very 
relevant. Either way I'll mark my request as resolved and let you decide which 
way to go.

[I edited the package names to include ".cache."]

[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org


[GitHub] [geode] kirklund commented on issue #2915: GEODE-6117: Makes modules out of geode-core and geode-cq

2018-12-06 Thread GitHub
I forgot geode-cq is a module but geode-query is not [edited]. It still seems 
like cq is part of query(ing) though. We can't use org.apache.geode.query.cq 
inside geode-cq? Or is that just convention? Would we ever create geode-query 
to contain org.apache.geode.query and then geode-cq adds cq onto that?

I'm thinking that geode-core will eventually contain org.apache.geode.logging 
to contain our LogService and SPI for the LogWriter and Alert appenders (or 
even move that to geode-logging). Then geode-log4j would contain our default 
impls of that SPI which are the LogWriter and Alert appenders that are written 
with log4j-core. Then geode-core uses geode-logging and geode-log4j is an 
optional provider to the SPI in geode-logging. Or is that too many sub-modules 
(I'm new to modules!)?

So, my input on cq is based mainly on cq being something that builds on query. 
If org.apache.geode.query.cq is ugly because of geode-cq or if we would never 
split out query to geode-query then my suggestion probably isn't very relevant. 
Either way I'll mark my request as resolved and let you decide which way to go.

[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org


[GitHub] [geode] kirklund commented on issue #2915: GEODE-6117: Makes modules out of geode-core and geode-cq

2018-12-06 Thread GitHub
I forgot about geode-cq as a module. It still seems like cq is part of 
query(ing) though. We can't use org.apache.geode.query.cq inside geode-cq? Or 
is that just convention? Would we ever create geode-query to contain 
org.apache.geode.query and then geode-cq adds cq onto that?

I'm thinking that geode-core will eventually contain org.apache.geode.logging 
to contain our LogService and SPI for the LogWriter and Alert appenders (or 
even move that to geode-logging). Then geode-log4j would contain our default 
impls of that SPI which are the LogWriter and Alert appenders that are written 
with log4j-core. Then geode-core uses geode-logging and geode-log4j is an 
optional provider to the SPI in geode-logging. Or is that too many sub-modules 
(I'm new to modules!)?

So, my input on cq is based mainly on cq being something that builds on query. 
If org.apache.geode.query.cq is ugly because of geode-cq or if we would never 
split out query to geode-query then my suggestion probably isn't very relevant. 
Either way I'll mark my request as resolved and let you decide which way to go.

[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org