-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add v5.0 INSPIRE Protected Sites SE files #36
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
Conversation
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.
Generally LGTM. I only added two comments related to formal aspects.
And one general comment on the SLDs v5.0:
When you do all the changes in se-files, it would be good if you do this based on the old se-files. For an easier review it would be good if you create the new se. files based on the existing v4.0 files. Commit these changes in one commit. Then do the respective changes due to the new schema version 5.0 and commit the respective changes in the se-files. With this the reviewer can see what changed in the se-file and see for instance if an adaptation was missed.
No need to do this for this PR here. But please consider for the further changes :)
@@ -0,0 +1,2 @@ | |||
eclipse.preferences.version=1 |
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.
If you want to add this config, please do it in a separate commit and remove it from this commit here (it's not related to the changes of PS-5.0)
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 have created a separate commit for the settings.
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.
One last thing, please mention in your commit message why you did this change. We adapted the settings because otherwise the "Umlaute" weren't displayed correctly.
And I hope I'm not wrong, but the correct prefix of such a commit should be "build:* and not "feat:".
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 have revised the commit messsage.
config.style
Outdated
@@ -1,4 +1,4 @@ | |||
tags: 'inspire' 'inspire_wetransform' 'production' | |||
tags: 'inspire' 'inspire_wetransform' 'production' |
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 assume this space here is not needed and that's only the change you did to trigger the automated workflow.
Please make sure to changes these temporarily adaptations back or make to not include them in your commit.
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 have edited it so that the space character is no longer included.
As part of a proof-of-concept to support both INSPIRE 4.0 and 5.0 in parallel on hale connect, the repository was extended with styling for the Protected Sites schema in version 5.0 by adding the file PS_ProtectedSites_v5.se, importing the ps 5.0 namespace, and creating the corresponding style and layer configurations, leaving the existing 4.0 setup fully intact. SVC-2136
6fb4743
to
0458acf
Compare
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.
LGTM, but ebfore merging the PR please adapt the commit message with the changes related to the eclipse settings (see my separate comment on that).
0458acf
to
92872e4
Compare
Add the eclipse settings regarding the version and encoding. This is done because otherwise "Umlaute" are not displayed correctly. SVC-2136
92872e4
to
21c0154
Compare
As part of a proof-of-concept to support both INSPIRE 4.0 and 5.0 in parallel on hale connect, the repository was extended with styling for the Protected Sites schema in version 5.0 by adding the file PS_ProtectedSites_v5.se, importing the ps 5.0 namespace, and creating the corresponding style and layer configurations, leaving the existing 4.0 setup fully intact.
SVC-2136