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

#20 Enable Non String values in JsonFieldPatternBuilder #20

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

RandySun228
Copy link
Contributor

Motivation

This PR is to address issue: #17.
JsonFieldPatternBuilder does not support non string data mask, so create this PR to fix it. Numeric and boolean data are supported.

Proposed Changes

  • add a new method buildPatternEntities in IPatternBuilder to support multiple regular expressions
  • deprecate existing buildPattern and buildReplacement
  • After this PR, we will also migrate existing JsonFullValuePatternBuilder, JsonRelativeFieldPatternBuilder and JsonValueUnmaskFromEndPatternBuilder, and then remove the above deprecated methods

Test Plan

Test is covered by UT

@prasanthkv prasanthkv changed the title enable Non String values in JsonFieldPatternBuilder #20 Enable Non String values in JsonFieldPatternBuilder Nov 8, 2024
prasanthkv
prasanthkv previously approved these changes Nov 19, 2024
Copy link
Collaborator

@prasanthkv prasanthkv left a comment

Choose a reason for hiding this comment

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

LGTM

@prasanthkv prasanthkv enabled auto-merge (squash) November 20, 2024 05:11
@prasanthkv prasanthkv added the enhancement New feature or request label Nov 20, 2024
@prasanthkv prasanthkv self-assigned this Nov 20, 2024
prasanthkv
prasanthkv previously approved these changes Nov 20, 2024
auto-merge was automatically disabled November 20, 2024 05:28

Head branch was pushed to by a user without write access

prasanthkv
prasanthkv previously approved these changes Nov 21, 2024
Copy link
Collaborator

@prasanthkv prasanthkv left a comment

Choose a reason for hiding this comment

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

@RandySun228 You need to sign your commits.

@RandySun228 RandySun228 force-pushed the master branch 2 times, most recently from b969783 to 828f6b1 Compare November 25, 2024 04:19
auto-merge was automatically disabled November 25, 2024 04:43

Pull request was closed

@RandySun228 RandySun228 merged commit 98455fa into eBay:master Nov 25, 2024
3 checks passed
Copy link
Collaborator

@prasanthkv prasanthkv left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants