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

#2 Support for Custom Ranges #4

Merged

Conversation

iv4n-ga6l
Copy link
Contributor

Allow users to specify custom ranges for the Int, Float, and Duration functions instead of always starting at 0.

@iv4n-ga6l iv4n-ga6l mentioned this pull request Sep 5, 2024
Copy link
Owner

@raphoester raphoester left a comment

Choose a reason for hiding this comment

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

fix conflicts with latest merge and it's good to go

chaos.go Outdated Show resolved Hide resolved
chaos.go Outdated Show resolved Hide resolved
chaos.go Outdated Show resolved Hide resolved
@iv4n-ga6l iv4n-ga6l requested a review from raphoester September 6, 2024 02:37
@raphoester raphoester force-pushed the specifyCustomRangesForIntFloatDuration branch from 9fc5419 to 0ba821a Compare September 6, 2024 10:04
@raphoester
Copy link
Owner

what do you think @IvanGael ? I adapted your ideas to the new package organization (with methods instead of plain functions)

@raphoester
Copy link
Owner

Maybe add some tests before merging ?

@iv4n-ga6l
Copy link
Contributor Author

It seems you change a little bit the implementation. The functions Int, MustIntInRange, Bool, Duration, MustDurationInRange, Time, Float, MustFloatInRange, ..Etc have been removed in chaos.go

Should I pull the latest changes from the main branch before adding some tests?

@raphoester
Copy link
Owner

Yea, chaos.go is only dedicated to the Chaos object now.
Functions and methods for native types have been moved to basic_types.go.
Therefore their tests should be moved to basic_types_test.go.

Also the behavior has been adapted to avoid unnecessary redundancy : Range functions become Between

@raphoester raphoester merged commit 206ddc5 into raphoester:main Sep 6, 2024
1 check passed
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.

2 participants