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

feat: add min max size for random slice map And allowing one element for Oneof #130

Merged
merged 7 commits into from Jan 18, 2022
Merged

feat: add min max size for random slice map And allowing one element for Oneof #130

merged 7 commits into from Jan 18, 2022

Conversation

autumn31
Copy link
Contributor

wish to add a feature that can set minimum random size for slice and map

@codecov
Copy link

codecov bot commented Feb 23, 2021 •
edited

Codecov Report

Merging #130 (981d489) into master (74585ae) will decrease coverage by 0.12%.
The diff coverage is 80.00%.

❗ Current head 981d489 differs from pull request most recent head 42898bb. Consider uploading reports for the commit 42898bb to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
- Coverage   92.17%   92.04%   -0.13%     
==========================================
  Files          11       11              
  Lines        1086     1094       +8     
==========================================
+ Hits         1001     1007       +6     
- Misses         46       47       +1     
- Partials       39       40       +1
Impacted Files Coverage Δ
faker.go 90.05% <80.00%> (-0.23%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5a6782...42898bb. Read the comment docs.

@makon makon mentioned this pull request May 6, 2021
Open
@bxcodec
Copy link
Owner

bxcodec commented Jul 1, 2021

Hi @autumn31 can you help to check the test, it's failing on the CI

Sorry for the late reply, been busy lately. 🙇🏻

@autumn31 autumn31 changed the title feat: add min max size for random slice map feat: add min max size for random slice map And allowing one element for Oneof Jul 2, 2021
@autumn31
Copy link
Contributor Author

autumn31 commented Jul 2, 2021 •
edited

@bxcodec Test fixed. Also I updated the mr title, since I added another feature that Oneof should accept "one element" cases.
Wonder how you think about this change?

bxcodec
bxcodec reviewed Dec 29, 2021
View reviewed changes
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 @autumn31

So sorry for the late reply.
Minor comment, otherwise LGTM

faker.go Outdated
if strings.Contains(argsList[0], ",,") {
return nil, fmt.Errorf(ErrDuplicateSeparator)
}
fmt.Printf("argsList(%v)\n", argsList[0])
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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, this line was removed in the latest commit.

bxcodec
bxcodec approved these changes Jan 18, 2022
View reviewed changes
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.

LGTM

@bxcodec bxcodec merged commit c826887 into bxcodec:master Jan 18, 2022
@bxcodec
Copy link
Owner

bxcodec commented Jan 18, 2022

Hey @autumn31 ,

Thanks for the PR, I'll release it soon. In the mean time, you can try to test on your project using the master branch for the source

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bxcodec bxcodec bxcodec approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@autumn31 @bxcodec