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.

Sign up for GitHub

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

Jump to bottom

Serialized key function #58

Merged
merged 9 commits into from
Nov 9, 2020
Merged

Serialized key function #58

merged 9 commits into from
Nov 9, 2020

Conversation

PizzaCatKing
Copy link
Collaborator

@PizzaCatKing PizzaCatKing commented Sep 24, 2020 •
edited
Loading

Fixes

fixes #7

Description

Adds the ability to use a property key transform function as a serialized key

Proposed changes in this PR

  • Adds ability to pass a function that dynamically transforms the property key into a serialized key

Things to look at

  • Test coverage
  • Code Style
  • Documentation (READMEs etc)

@PizzaCatKing PizzaCatKing self-assigned this Sep 24, 2020
@hardy925
Copy link
Contributor

@PizzaCatKing thoughts on this pr?

@hardy925 hardy925 added enhancement New feature or request question Further information is requested v1.0.0 roadmap labels Nov 4, 2020
@hardy925
Copy link
Contributor

hardy925 commented Nov 6, 2020

hey @PizzaCatKing what is left for this PR? do we need more tests or docs?

hardy925
hardy925 reviewed Nov 6, 2020
View reviewed changes
serialize_property.ts Outdated Show resolved Hide resolved
@PizzaCatKing PizzaCatKing marked this pull request as ready for review November 8, 2020 00:04
ms1111
ms1111 approved these changes Nov 8, 2020
View reviewed changes
Copy link
Collaborator

@ms1111 ms1111 left a comment

Choose a reason for hiding this comment

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

I figure this layers on top of #66, so #66 lets you specify a class-level function for serialized keys and this one lets you specify a property-level function. The order of precedence doesn't change: you can specify a string or function at the property level, and if you don't, the class-level tsTransformKey() will be used.

hardy613
hardy613 reviewed Nov 8, 2020
View reviewed changes
Copy link
Collaborator

@hardy613 hardy613 left a comment

Choose a reason for hiding this comment

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

I don't think we'll need ToSerializedKeyStrategy for node right,

this is awesome though.

hardy925
hardy925 approved these changes Nov 9, 2020
View reviewed changes
Copy link
Contributor

@hardy925 hardy925 left a comment

Choose a reason for hiding this comment

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

LGTM! thank you

@hardy925 hardy925 merged commit bbd5834 into develop Nov 9, 2020
@hardy925 hardy925 deleted the serialized-key-function branch November 9, 2020 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hardy613 hardy613 hardy613 left review comments

@hardy925 hardy925 hardy925 approved these changes

@ms1111 ms1111 ms1111 approved these changes

@ChrisDufourMB ChrisDufourMB Awaiting requested review from ChrisDufourMB ChrisDufourMB is a code owner

Assignees

@PizzaCatKing PizzaCatKing

Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC] - Allow property name to take a function in addition to a fixed string
4 participants
@PizzaCatKing @hardy925 @hardy613 @ms1111