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

Use unique feature id from tileset if provided #463

Merged
merged 4 commits into from
Sep 2, 2022

Conversation

bbecquet
Copy link
Contributor

@bbecquet bbecquet commented Sep 1, 2022

Description

Shortcut: https://app.shortcut.com/cartoteam/story/253930/unique-id-property-from-fields

Modify the feature filter for tiles to take into account the newly provided explicit unique ID, so features are properly identified and not counted multiple times if they intersect multiple tiles.

Type of change

  • Fix

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #253930: Unique id property from fields.

@bbecquet bbecquet requested a review from padawannn September 1, 2022 08:53
@coveralls
Copy link
Collaborator

coveralls commented Sep 1, 2022

Pull Request Test Coverage Report for Build 2977824999

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 72.447%

Totals Coverage Status
Change from base Build 2805103931: 0.03%
Covered Lines: 1658
Relevant Lines: 2145

💛 - Coveralls

@bbecquet bbecquet force-pushed the feature/sc-253930/unique-id-property-from-fields branch from 0c45464 to db60905 Compare September 1, 2022 13:01
});

// prettier-ignore
expect(properties).toEqual([
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice test here @bbecquet 👍🏻

@VictorVelarde VictorVelarde self-requested a review September 1, 2022 19:18
Copy link
Contributor

@VictorVelarde VictorVelarde left a comment

Choose a reason for hiding this comment

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

I think changes make sense. Confirm with @padawannn and please go ahead with the release

Copy link
Contributor

@padawannn padawannn left a comment

Choose a reason for hiding this comment

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

Good work

@bbecquet bbecquet merged commit 364fb4b into master Sep 2, 2022
@bbecquet bbecquet deleted the feature/sc-253930/unique-id-property-from-fields branch September 2, 2022 10:04
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.

4 participants