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

[BitSail][Feature] Support generate constant value in FakeSource #341

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kyle-hawk
Copy link
Contributor

Signed-off-by:

Pre-Checklist

Note: Please complete ALL items in the following checklist.

  • I have read through the CONTRIBUTING.md documentation.
  • My code has the necessary comments and documentation (if needed).
  • I have added relevant tests.

Purpose

Approaches

users can config constant value by defaultValue field, like this

      "columns": [
        {
          "name": "double_value",
          "type": "double",
          "defaultValue": "13.5"
        }
      ]

Related Issues

New Behavior (screenshots if needed)

N/A

@@ -159,4 +173,107 @@ private Object fakeRawValue(TypeInfo<?> typeInfo) {
}
throw new RuntimeException("Unsupported type " + typeInfo);
}

@SuppressWarnings("checkstyle:MagicNumber")
private Object constantRawValue(TypeInfo<?> typeInfo, String constantValue) {
Copy link
Member

Choose a reason for hiding this comment

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

It seems that the code is redundant. Why not combine fakeRawValue and constantRawValue into one function

Copy link
Contributor Author

@kyle-hawk kyle-hawk Feb 2, 2023

Choose a reason for hiding this comment

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

thx for ur advice.
When I was writing, I also thought about whether to implement fakeRawValue and constantRawValue in one function, I thought fakeRawValue would be too long and hard to maintain for different logic mix together after combining, so I separated them, and the cost is that we got 2 functions that look similar

Copy link
Member

Choose a reason for hiding this comment

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

After the merger, the code will be more concise, looking forward to your contribution @kyle-hawk

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, thx for the guidance, I'll merge them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lichang-bd I have merged them. please take a look. thx

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.

[BitSail][Feature] Support generate constant value in FakeSource.
2 participants