Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Imagery definitions refactor #1

Closed
wants to merge 25 commits into from

Conversation

wiktorn
Copy link
Owner

@wiktorn wiktorn commented May 3, 2018

wiktorn and others added 11 commits May 1, 2018 23:03
- moveReaderToTag - now you can specify function which checks if tags
are the same
- getElementTextWithSubtags - returns the content of tag including all
tags that may exist within
* make layer classes public
* use WMTSCapabilities to pass general data (independent of layers)
* make getCapabilities static
* throw an exception if data is missing in document
* use SAX parser for WMS
* use common parts with WMTS parser
* allow setting custom headers
* allow setting default layer
@wiktorn wiktorn changed the title Imager definitions refactor Imagery definitions refactor May 3, 2018
return layerName;
}

public String getTileMatrixSet() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add Javadoc for all public methods?

if (directory.length() + fileName.length() > 255) {
// Windows doesn't support paths longer than 260, leave 5 chars as safe buffer, 4 will be used by ".tmp"
// TODO: what about filename size on other systems? 255?
if (directory.length() > 191 && System.getProperty("os.name").toLowerCase().indexOf("win") >= 0) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use Main.isPlatformWindows()

} catch (NoSuchAlgorithmException e) {
Logging.error(e);
// TODO: what better can we do here?
throw new IllegalArgumentException("Missing digest algorithm SHA-256");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chain e

@@ -111,71 +112,125 @@ public WMSGetCapabilitiesException(String message, String incomingData) {

/**
* The data that caused this exception.
* @return The server response to the capabilities request.
* @return The server response to the capabilites request.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

@@ -1,86 +1,86 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file should not be modified

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only EOL change to unix.

}

@Test
@Ignore("disabled as this needs user action") // XXX
//@Ignore("disabled as this needs user action") // XXX
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the test does not need user action any more, the line should be deleted, not commented

"</imagery>"
)));
mapsMock.start();
// Main.pref.set
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line to be removed

assertTrue(wms.getLayers().get(0).getAbstract().startsWith("South Carolina NAIP Imagery 2017 Resolution: 100CM "));
wm.shutdown();
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank lines to remove

@wiktorn
Copy link
Owner Author

wiktorn commented May 31, 2018

merged

@wiktorn wiktorn closed this May 31, 2018
@wiktorn wiktorn deleted the imager_definitions_refactor_review branch May 31, 2018 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants