-
-
Notifications
You must be signed in to change notification settings - Fork 147
feat: add v4 initial version, with options and restructured package #169
Conversation
example_single_fake_data_test.go
Outdated
|
||
// Single fake function can be used for retrieving particular values. | ||
func Example_singleFakeData() { | ||
|
||
// Address | ||
faker.Latitude() // => 81.12195 | ||
faker.Longitude() // => -84.38158 | ||
faker.Latitude(options.DefaultOption()) // => 81.12195 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now every function got a required param? I think optional param (like ...options.OptionFunc
) is a better idea, It's easier to upgrade from V3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR updated. please help to re-review again
* feat: update WithRecursionMaxDepth & unittest * style: remove unnecessary check
@wolf-joe can you help to re-review again? I decided to keep Options publicly available, the reason is just it's the faker option, and people can read it as they want. |
} | ||
|
||
// SetRandomMapAndSliceMaxSize sets the max size for maps and slices for random generation. | ||
func SetRandomMapAndSliceMaxSize(size int) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep those set function, it provides global setting ability and back forward compatibility
There was a problem hiding this 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 this is possible, since we remove the global variables, and use options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And since we're introducing major version, breaking changes is acceptable.
And no need to be 100% backward compatibility
I'm merging this now and will release a v4.0.0-beta version. |
fix #153
fix #144
fix #114