Closed LukeL-dev closed 8 years ago
Reviewed 82 of 167 files at r1, 11 of 17 files at r2. Review status: 93 of 183 files reviewed at latest revision, 4 unresolved discussions.
GeoPackage/src/main/java/com/rgi/geopackage/features/BinaryHeader.java, line 114 [r3] (raw file): envelop's should be envelope's*
Comments from Reviewable
Reviewed 1 of 167 files at r1. Review status: 94 of 183 files reviewed at latest revision, 5 unresolved discussions.
GeoPackage/src/main/java/com/rgi/geopackage/features/BinaryType.java, line 39 [r3] (raw file): remove todo. Not obvious what these are, please add comment :)
Comments from Reviewable
Reviewed 1 of 167 files at r1. Review status: 20 of 117 files reviewed at latest revision, 6 unresolved discussions.
_GeoPackage/src/main/java/com/rgi/geopackage/features/ByteOutputStream.java, line 32 [r3] (raw file):_ Not many comments in this class including on public methods. It would be a good idea to add some though most of it is obvious. Just a suggestion
Comments from Reviewable
Reviewed 6 of 167 files at r1. Review status: 26 of 117 files reviewed at latest revision, 8 unresolved discussions.
_GeoPackage/src/main/java/com/rgi/geopackage/features/FeatureSet.java, line 92 [r3] (raw file):_ may not be null OR empty*
_GeoPackage/src/main/java/com/rgi/geopackage/features/FeatureSet.java, line 97 [r3] (raw file):_ may not be null OR empty*
Comments from Reviewable
Reviewed 74 of 167 files at r1, 5 of 17 files at r2, 3 of 3 files at r3, 9 of 79 files at r4. Review status: all files reviewed at latest revision, 12 unresolved discussions.
[GeoPackage/src/main/java/com/rgi/geopackage/features/GeometryType.java, line 32 [r3]](https://reviewable.io:443/reviews/githubrgi/swagd/382#-KH1P35ykoKNWFyU1jZs:-KH1P362n6vvQGVJ9lS:567759719) (raw file):_ did you mean to leave this here?
GeoPackage/src/main/java/com/rgi/geopackage/features/GeoPackageFeatures.java, line 868 [r4] (raw file): maybe mention if the range is inclusive or not inclusive
_GeoPackage/src/test/java/com/rgi/geopackage/features/SqlTypeTest.java, line 49 [r4] (raw file):_ Can't this class be ignored due to it being an enum? So that it won't effect the coverage
_GeoPackage/src/test/java/com/rgi/geopackage/features/ValueRequirementTest.java, line 78 [r4] (raw file):_ instead of catching the IAE have (expected = IllegalArgumentException.class) next to the annotation(@Test) this will make the junit test fail if this exception is not thrown. Then this can be split into different tests.
Comments from Reviewable
This change is