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

Restore early lake labels, add kind_detail for lakes and etc #2047

Merged
merged 27 commits into from
Feb 1, 2022

Conversation

nvkelso
Copy link
Member

@nvkelso nvkelso commented Jan 20, 2022

Connects with #2003 and followup of #2010 to restore "lake" water labels at low zooms.

Also non-breaking change implementation of #984 for the v1.x series (which should stay open as a separate breaking change for v2.x series).

  • Update tests
  • Update docs

@peitili
Copy link
Contributor

peitili commented Jan 26, 2022

there is a linter violations, you might want to fix them using the pre-commit

@peitili
Copy link
Contributor

peitili commented Jan 26, 2022

I am not sure why but the integration tests always failed with context deadline exceed.

Copy link
Member Author

@nvkelso nvkelso left a comment

Choose a reason for hiding this comment

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

(moved to comment in PR review instead)

@nvkelso nvkelso changed the title WIP: Restore early lake labels Restore early lake labels, add kind_detail for lakes and etc Jan 28, 2022
@@ -9471,6 +9470,9 @@ def update_min_zoom(ctx):
local = defaultdict(lambda: None)
local.update(props)
local['zoom'] = zoom
# this is to make the name `properties` visible in the queries.yaml's
# where clause
local['properties'] = props
Copy link
Member Author

Choose a reason for hiding this comment

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

This may impact build time, but generally makes things more consistent and obvious.

@peitili peitili force-pushed the nvkelso/water-lake-labels-fix branch from 0b54f17 to 912b486 Compare February 1, 2022 00:51
Copy link
Member Author

@nvkelso nvkelso left a comment

Choose a reason for hiding this comment

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

Final set of code, tests, and docs LGTM, including Q&A of a mini build.

@peitili this is ready to merge

@peitili peitili merged commit 215fd7a into master Feb 1, 2022
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.

3 participants