Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

fix slice tag bug AND add chinese name #141

Merged
merged 20 commits into from Jan 31, 2022
Merged

Conversation

qiangmzsx
Copy link
Contributor

fix bug

AdCode[]string `json:"ad_code" xorm:"ad_code" faker:"len=slice_len=5,len=10"` 

return:

{AdCode:[rMZLe XgkhW IZHBZ qEaWa nwrKJ]}

but

AdCode[]string `json:"ad_code" xorm:"ad_code" faker:"len=10,slice_len=5"`

return:

{AdCode:[RotjXUZmnq MTbKBCQhJC PedUsSqIwv xTnlUgWajY PBENmHwGfc]}

features

  • add chinese_first_name
  • add chinese_last_name
  • add chinese_name

@qiangmzsx
Copy link
Contributor Author

I don't see the previously committed code being merged into master, is there anything I need to do?

@bxcodec
Copy link
Owner

bxcodec commented Jul 1, 2021

Sorry, there's a linter issue. Not sure why, it's passed before merged to master, but it failed now.

@bxcodec
Copy link
Owner

bxcodec commented Jul 1, 2021

Can you help to fix the linter?

@qiangmzsx
Copy link
Contributor Author

I will deal with it as soon as possible and submit the code

@qiangmzsx
Copy link
Contributor Author

Through testing I found some problems with the code I wrote before and fixed them. Now it should be ready for merging, thanks!

Copy link
Owner

@bxcodec bxcodec left a comment

Choose a reason for hiding this comment

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

Hey @qiangmzsx

So sorry for the late reply, I was so busy in the past few months, just had time to review your PR now.

I have a few comments, please re-review when you have time, thnx

faker.go Outdated
}

// fmt.Println("++TAGS:", tags, res)
Copy link
Owner

Choose a reason for hiding this comment

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

can remove this?

faker_test.go Outdated
@@ -1954,6 +2020,7 @@ func TestOneOfTag__BadInputsForInts(t *testing.T) {
err := FakeData(&a)
if err == nil {
t.Fatal("expected error, but got no error")
return
Copy link
Owner

Choose a reason for hiding this comment

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

why need to return? Is there any issue?

@qiangmzsx
Copy link
Contributor Author

Thanks for the review. I have made the changes and would appreciate your review again.

@bxcodec
Copy link
Owner

bxcodec commented Jan 27, 2022

Hey @qiangmzsx I think it's conflict, can you rebase your code from the master, and apply your changes?

@qiangmzsx
Copy link
Contributor Author

I'm very sorry! This was my problem, caused by not doing a pull of the latest code for a long time, and I have fixed it. Request another review.

@@ -134,6 +137,15 @@ const (
//hyphen = "-"
)

// PriorityTags define the priority order of the tag
var PriorityTags = []string{ID, HyphenatedID, EmailTag, MacAddressTag, DomainNameTag, UserNameTag, URLTag, IPV4Tag,
Copy link
Owner

@bxcodec bxcodec Jan 27, 2022

Choose a reason for hiding this comment

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

remind me again, what are these priority tags used for?

@@ -653,7 +673,28 @@ func decodeTags(typ reflect.Type, i int) structTag {
uni = true
continue
}
res = append(res, tag)
// res = append(res, tag)
Copy link
Owner

Choose a reason for hiding this comment

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

please remove if not used anymore

@bxcodec
Copy link
Owner

bxcodec commented Jan 27, 2022

Hey @qiangmzsx
reviewed, a minor change, also I have a question. Please address when you're free thank you 🙇🏻

@qiangmzsx
Copy link
Contributor Author

I'm online today, so feel free to communicate with me if you need anything.

@qiangmzsx
Copy link
Contributor Author

My email address is qiangmzsx@hotmail.com

@bxcodec bxcodec merged commit cabb112 into bxcodec:master Jan 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants