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

Add shields for Croatia #1056

Merged
merged 5 commits into from
May 10, 2024
Merged

Add shields for Croatia #1056

merged 5 commits into from
May 10, 2024

Conversation

Janjko
Copy link
Contributor

@Janjko Janjko commented May 7, 2024

Added shields for Autoceste, Državne ceste, Županijske ceste and Lokalne ceste.

@ZeLonewolf
Copy link
Member

Preview:
image

@ZeLonewolf
Copy link
Member

ZeLonewolf commented May 7, 2024

Note that the A3 shield is currently doubled because the two sides of the highway have slightly different name tags. When they get coalesced, they're therefore treated as separate routes. I would consider that a tagging error that should not hold this up.
(tile server location)

Copy link
Member

@ZeLonewolf ZeLonewolf left a comment

Choose a reason for hiding this comment

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

This looks great, thank you for the contribution!

@claysmalley
Copy link
Member

Thanks for your contribution! Don't forget npm run status_map to highlight Croatia on the shield coverage map.

@Janjko
Copy link
Contributor Author

Janjko commented May 7, 2024

Note that the A3 shield is currently doubled because the two sides of the highway have slightly different name tags. When they get coalesced, they're therefore treated as separate routes. I would consider that a tagging error that should not hold this up. (tile server location)

It looks great, thanks!

I'll see what are standards with these double routes, they will probably lose a few tags soon.

@1ec5
Copy link
Member

1ec5 commented May 7, 2024

Note that the A3 shield is currently doubled because the two sides of the highway have slightly different name tags. When they get coalesced, they're therefore treated as separate routes. I would consider that a tagging error that should not hold this up.

This will happen anywhere a route relation’s name has a disambiguating suffix or other description, right?

@1ec5 1ec5 linked an issue May 7, 2024 that may be closed by this pull request
@ZeLonewolf
Copy link
Member

Note that the A3 shield is currently doubled because the two sides of the highway have slightly different name tags. When they get coalesced, they're therefore treated as separate routes. I would consider that a tagging error that should not hold this up.

This will happen anywhere a route relation’s name has a disambiguating suffix or other description, right?

Yes, if they're different. This was one of the motivations for the linked edits.

Copy link
Member

@ZeLonewolf ZeLonewolf left a comment

Choose a reason for hiding this comment

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

@Janjko please run npm run status_map and commit the results

@Janjko
Copy link
Contributor Author

Janjko commented May 9, 2024

@Janjko please run npm run status_map and commit the results

I wasn't able to do this. I ran it in a docker with the newest node (v22.1.0) and running npm run status_map results in:
Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@americana/maplibre-shield-generator' imported from /src/src/js/shield_defs.js

Then running npm install @americana/maplibre-shield-generator results in:

root@43e18cb7f75c:/src# npm install @americana/maplibre-shield-generator
npm warn deprecated [email protected]: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
npm warn deprecated [email protected]: Package no longer supported. Contact Support at https://www.npmjs.com/support for more info.
npm error code 1
npm error path /src/node_modules/canvas
npm error command failed
npm error command sh -c node-pre-gyp install --fallback-to-build --update-binary
npm error make: Entering directory '/src/node_modules/canvas/build'
npm error   SOLINK_MODULE(target) Release/obj.target/canvas-postbuild.node
npm error   COPY Release/canvas-postbuild.node
npm error   CXX(target) Release/obj.target/canvas/src/backend/Backend.o
npm error make: Leaving directory '/src/node_modules/canvas/build'
npm error Failed to execute '/usr/local/bin/node /usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js build --fallback-to-build --update-binary --module=/src/node_modules/canvas/build/Release/canvas.node --module_name=canvas --module_path=/src/node_modules/canvas/build/Release --napi_version=9 --node_abi_napi=napi --napi_build_version=0 --node_napi_label=node-v127' (1)
...

Then I tried to install canvas, but that resulted in further errors, so I tried to install:
apt-get install libcairo2-dev libjpeg-dev libpango1.0-dev libgif-dev build-essential g++

And that succeeded, but didn't help with installing canvas, and so on..

So I failed that task..

@claysmalley
Copy link
Member

Just open this file in a text editor and add one line between Greece and Iceland:

.gr,
.hr,
.is,

It will do the exact same thing as npm run status_map, without fussing with dependencies.

@Janjko
Copy link
Contributor Author

Janjko commented May 10, 2024

There, added .hr

@ZeLonewolf ZeLonewolf self-requested a review May 10, 2024 12:23
@ZeLonewolf ZeLonewolf merged commit f55304f into osm-americana:main May 10, 2024
6 checks passed
@Janjko Janjko deleted the patch-1 branch May 16, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Shields of Croatia
4 participants