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

Move signals for each country into their own files #127

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

besentv
Copy link
Contributor

@besentv besentv commented Apr 7, 2024

Like the title says, the goal is to move signalling definitions for each country into their own files for easier navigation, and addition of future signalling systems.

Additionally, I added a license header to each file, like it's suggested by the project's license: https://github.com/OpenRailwayMap/OpenRailwayMap-CartoCSS/blob/master/COPYING#L623

@DerDakon
Copy link
Contributor

This is a good move, but I don't like 2 things. First I would suggest to use short SPDX identifiers instead of copying the license text into each file. And given that you only moved lines around I don't see any reason that you should add your copyright into these files.

@besentv
Copy link
Contributor Author

besentv commented Apr 14, 2024

First I would suggest to use short SPDX identifiers instead of copying the license text into each file. And given that you only moved lines around I don't see any reason that you should add your copyright into these files.

Sure, I'll be changing it. :)

@besentv
Copy link
Contributor Author

besentv commented Apr 14, 2024

Done.

Copy link
Collaborator

@Nakaner Nakaner left a comment

Choose a reason for hiding this comment

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

Some German and Austrian signals look identical and use the same styling rules. I would like to ask to move these signals to a file called signals_multiple_countries.mss (or a different name if you find a better one).

This applies for following signals:

  • DE-ESO:ra10, AT-V2:verschubhalttafel
  • DE-ESO:ra11, AT-V2:wartesignal
  • DE-ESO:ne1, AT-V2:trapeztafel
  • DE-ESO:so16, AT-V2:kreuztafel
  • semaphore versions of DE-ESO:hp and AT-V2:hauptsignal

@besentv
Copy link
Contributor Author

besentv commented May 16, 2024

Some German and Austrian signals look identical and use the same styling rules. I would like to ask to move these signals to a file called signals_multiple_countries.mss (or a different name if you find a better one).

Did a general rebase and am going to apply this later.

@besentv
Copy link
Contributor Author

besentv commented Jun 4, 2024

I gave this some thoughts, but I'm not sure if I agree on adding a stylesheet for signals that share the same rules. The idea here is to have no code duplication, which makes perfectly sense, but adding a common file sort of defeats the purpose of splitting the code in the first place. Unfortunately, CartoCSS doesn't have a C or SASS style preprocessor. One proposal I could make is to have a makefile/python script that does that for us, or writing a custom version of the CartoCSS compiler.
So we'd end up having something like:

signals_common.mss:

@mixin shunting-stop() {
    marker-file: url('symbols/de/ra10.svg');
    marker-width: 16;
    marker-height: 11;
    marker-allow-overlap: true;
}

signals_at.mss:

[zoom>=17]["feature"="AT-V2:verschubsignal"]["shunting_form"="light"] {
    @include shunting-stop();
}

signals_de.mss:

[zoom>=17]["feature"="DE-ESO:ra10"]["shunting_form"="sign"] {
    @include shunting-stop();
}

@besentv
Copy link
Contributor Author

besentv commented Oct 23, 2024

@Nakaner Any thoughts on this?

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.

3 participants