-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
add extraMetadata to profile interface [#794] #795
Conversation
https://github.com/onthegomap/planetiler/actions/runs/7554493975 ℹ️ Base Logs 0cb2645
ℹ️ This Branch Logs c23e3b6
|
planetiler-core/src/main/java/com/onthegomap/planetiler/archive/TileArchiveMetadata.java
Outdated
Show resolved
Hide resolved
@@ -157,6 +158,8 @@ default boolean isOverlay() { | |||
return false; | |||
} | |||
|
|||
default Map<String,String> extraMetadata() { return Map.of(); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should include archive
in the name here to make it clear where the metadata is going?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to extraArchiveMetadata
788515b
to
88c0746
Compare
* simpler implementation of mergeMaps
@@ -93,7 +96,7 @@ public TileArchiveMetadata(Profile profile, PlanetilerConfig config, List<LayerA | |||
config.minzoom(), | |||
config.maxzoom(), | |||
vectorLayers == null ? null : new TileArchiveMetadataJson(vectorLayers), | |||
mapWithBuildInfo(), | |||
mergeMaps(mapWithBuildInfo(),profile.extraArchiveMetadata()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd expect the profile to provide defaults, then args to layer on top of that, so let's either reverse this or the behavior of mergeMaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't mapWithBuildInfo
just be the planetiler:*
metadata? If I define those in my profile (or command line args passed into my profile), they ought to override the defaults, I don't know why someone would do that though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right! Yeah this looks good then. The command-line args get copied in just above this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! let's just remove the unused imports then good to merge.
@@ -93,7 +96,7 @@ public TileArchiveMetadata(Profile profile, PlanetilerConfig config, List<LayerA | |||
config.minzoom(), | |||
config.maxzoom(), | |||
vectorLayers == null ? null : new TileArchiveMetadataJson(vectorLayers), | |||
mapWithBuildInfo(), | |||
mergeMaps(mapWithBuildInfo(),profile.extraArchiveMetadata()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right! Yeah this looks good then. The command-line args get copied in just above this.
import java.util.stream.Collectors; | ||
import java.util.stream.Stream; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove unused imports
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
No description provided.