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

New Release v1.5.2 - #patch #94

Merged
merged 5 commits into from
Nov 18, 2024
Merged

New Release v1.5.2 - #patch #94

merged 5 commits into from
Nov 18, 2024

Conversation

sami-nouidri-swisstopo
Copy link
Contributor

The deployment on DEV has been done and tested successfully. As such, I want to merge develop into master to conduct integration tests with the map viewer.

For reminder, this patch removes UNLISTED_ICON_SETS' default value, such that only the k8s configuration defines what sets are unlisted.

Sami Nouidri and others added 5 commits November 13, 2024 17:15
This change helps us avoid changing UNLISTED_ICON_SET's content on both
the service and the k8s configuration.

In order to achieve this, I'm assigning an empty string '' (since
os.environ.get requires a default value), and removing that string with a
simple loop.
This change improves readability of settings.py, and removes the double
assignement, with a cleaner overall routine.

UNLISTED_ICON_SETS is now assigned in settings.py using the function
fetch_and_clean_unlisted_sets.
Previously, fetch_and_clean_unlisted_sets was specifically only usable to
fetch an environment variable and clean it.

In order to address this, this workflow has been separated. In settings.py,
the environment variable is fetched and passed onto split_and_clean_string.
…isted-value

PB-1083: Removed default icon set 'babs' from UNLISTED_ICON_SET
Copy link
Contributor

@ltkum ltkum left a comment

Choose a reason for hiding this comment

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

lgtm

@sami-nouidri-swisstopo sami-nouidri-swisstopo merged commit fe52217 into master Nov 18, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants