Hey all,
I've been getting hands dirty with the wicket UI finally and would like to comment on some stuff.
I have refactored web.data.datastore and web.data.coverage into a single package web.data.store as the pages are about configuring the StoreInfo classes, not DataStore/CoverageStore.
Am also splitting out the concrete store pages from a single page to add/edit into separate pages both to follow naming convention FooNewPage/FooEditPage and to avoid a single class having two responsibilities.
While in the process found that both DataStoreInfo and CoverageStoreInfo new/edit pages completely lacked unit tests, so I also started adding them, since they're quite buggy when not on the everything-goes-fine use case, and also catched on some non tested behaviour about the switch to use real identifiers instead of name as id on the catalog objects.
Also found some cross references between packages that I would like to avoid. For instance, the package ...web.data contains all the resource configuration pages as well as the packages for configuring layer, layergroup, store, workspace and style.
I would like to move the resource configuration pages to a new data.resource subpackage so the data package interface gets clean. Another change would be to move the NewDataPage to the store package since it is only referenced by the store package and then NewDataPage references classes back in the store package.
So, any objection about those changes? am I missing something?
Cheers,
Gabriel
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.
Gabriel Roldan ha scritto:
Hey all,
I've been getting hands dirty with the wicket UI finally and would like to comment on some stuff.
I have refactored web.data.datastore and web.data.coverage into a single package web.data.store as the pages are about configuring the StoreInfo classes, not DataStore/CoverageStore.
Am also splitting out the concrete store pages from a single page to add/edit into separate pages both to follow naming convention FooNewPage/FooEditPage and to avoid a single class having two responsibilities.
While in the process found that both DataStoreInfo and CoverageStoreInfo new/edit pages completely lacked unit tests, so I also started adding them, since they're quite buggy when not on the everything-goes-fine use case, and also catched on some non tested behaviour about the switch to use real identifiers instead of name as id on the catalog objects.
Yeah, we did schedule full test coverage for after the 2.0alpha2 release, along with a raft of cosmetic improvements:
http://jira.codehaus.org/browse/GEOS-2704
The first part of the effort was to get the UI going for the
working case, that alone was quite an effort, nothing in the existing configuration pages was actually working at all.
Also found some cross references between packages that I would like to avoid. For instance, the package ...web.data contains all the resource configuration pages as well as the packages for configuring layer, layergroup, store, workspace and style.
I would like to move the resource configuration pages to a new data.resource subpackage so the data package interface gets clean. Another change would be to move the NewDataPage to the store package since it is only referenced by the store package and then NewDataPage references classes back in the store package.
So, any objection about those changes? am I missing something?
No objections here, just make sure to coordinate with Justin
for the next alpha release (wasn't that scheduled this week?)
Cheers
Andrea
Andrea Aime wrote:
Gabriel Roldan ha scritto:
Hey all,
I've been getting hands dirty with the wicket UI finally and would like to comment on some stuff.
I have refactored web.data.datastore and web.data.coverage into a single package web.data.store as the pages are about configuring the StoreInfo classes, not DataStore/CoverageStore.
Am also splitting out the concrete store pages from a single page to add/edit into separate pages both to follow naming convention FooNewPage/FooEditPage and to avoid a single class having two responsibilities.
While in the process found that both DataStoreInfo and CoverageStoreInfo new/edit pages completely lacked unit tests, so I also started adding them, since they're quite buggy when not on the everything-goes-fine use case, and also catched on some non tested behaviour about the switch to use real identifiers instead of name as id on the catalog objects.
Yeah, we did schedule full test coverage for after the 2.0alpha2 release, along with a raft of cosmetic improvements:
http://jira.codehaus.org/browse/GEOS-2704
The first part of the effort was to get the UI going for the
working case, that alone was quite an effort, nothing in the existing configuration pages was actually working at all.
Understood, I just added a couple tests for datastore creation as it was being caught by the changes in id handling without notice and nothing was working again. And for the record, no finger pointing here, after all I'm the one that made that page in the first place
Also found some cross references between packages that I would like to avoid. For instance, the package ...web.data contains all the resource configuration pages as well as the packages for configuring layer, layergroup, store, workspace and style.
I would like to move the resource configuration pages to a new data.resource subpackage so the data package interface gets clean. Another change would be to move the NewDataPage to the store package since it is only referenced by the store package and then NewDataPage references classes back in the store package.
So, any objection about those changes? am I missing something?
No objections here, just make sure to coordinate with Justin
for the next alpha release (wasn't that scheduled this week?)
Justin are you ok with moving resource config to its own package instead of being held in the parent "data" one whilst all the other config objects have their own packages inside "data"? it should be a 10 mins fix with manually testing included so I'd like to make it now while I'm on it.
Cheers,
Gabriel
Cheers
Andrea
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.
Gabriel Roldan wrote:
Andrea Aime wrote:
Gabriel Roldan ha scritto:
Hey all,
I've been getting hands dirty with the wicket UI finally and would like to comment on some stuff.
I have refactored web.data.datastore and web.data.coverage into a single package web.data.store as the pages are about configuring the StoreInfo classes, not DataStore/CoverageStore.
Am also splitting out the concrete store pages from a single page to add/edit into separate pages both to follow naming convention FooNewPage/FooEditPage and to avoid a single class having two responsibilities.
While in the process found that both DataStoreInfo and CoverageStoreInfo new/edit pages completely lacked unit tests, so I also started adding them, since they're quite buggy when not on the everything-goes-fine use case, and also catched on some non tested behaviour about the switch to use real identifiers instead of name as id on the catalog objects.
Yeah, we did schedule full test coverage for after the 2.0alpha2 release, along with a raft of cosmetic improvements:
http://jira.codehaus.org/browse/GEOS-2704
The first part of the effort was to get the UI going for the
working case, that alone was quite an effort, nothing in the existing configuration pages was actually working at all.
Understood, I just added a couple tests for datastore creation as it was being caught by the changes in id handling without notice and nothing was working again. And for the record, no finger pointing here, after all I'm the one that made that page in the first place
Also found some cross references between packages that I would like to avoid. For instance, the package ...web.data contains all the resource configuration pages as well as the packages for configuring layer, layergroup, store, workspace and style.
I would like to move the resource configuration pages to a new data.resource subpackage so the data package interface gets clean. Another change would be to move the NewDataPage to the store package since it is only referenced by the store package and then NewDataPage references classes back in the store package.
So, any objection about those changes? am I missing something?
No objections here, just make sure to coordinate with Justin
for the next alpha release (wasn't that scheduled this week?)
Justin are you ok with moving resource config to its own package instead of being held in the parent "data" one whilst all the other config objects have their own packages inside "data"? it should be a 10 mins fix with manually testing included so I'd like to make it now while I'm on it.
Yup, go ahead and make the changes. But keep in mind that their are lots of naming and packaging improvements to be made. So I don't want to go down this road too far so that all this refactoring becomes a hurdle for the release. But proceed with the changes, +1.
Cheers,
Gabriel
Cheers
Andrea
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.