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

Custom rules separator #25

Merged
merged 1 commit into from
Jul 5, 2022
Merged

Conversation

admssa
Copy link
Contributor

@admssa admssa commented Jul 4, 2022

In order to be able to use more complex rules.
Some rules may contain commas but rules_as_list = rules_as_string.split(',') split it and break.

@Andrey9kin
Copy link
Member

@admssa Thanks for the pull request! Doing separator configurable is definitely a good idea. Though there are two distinct changes bundled together in the same pull request. One is configurable separators and another one is a non-backward compatible change in the interface. Would it be too much trouble to submit a configurable separator as a separate pull request? We will merge it straight away.
As for the interface change. Yes, it is better to have it as a list of strings but at the same time, I would prefer keeping the old variable (that takes input as a string) in place and marking it deprecated mean while providing a new way to provide rules a list of strings. In this way, we give people time to prepare for the change while not blocking upgrades to the new versions. What do you think?

@Andrey9kin Andrey9kin self-assigned this Jul 4, 2022
@Andrey9kin Andrey9kin added the enhancement New feature or request label Jul 4, 2022
@admssa
Copy link
Contributor Author

admssa commented Jul 5, 2022

@Andrey9kin Updated. Kept the separator only.

@Andrey9kin Andrey9kin merged commit 61a3275 into fivexl:master Jul 5, 2022
@Andrey9kin
Copy link
Member

@admssa perfect! Thanks!

@admssa
Copy link
Contributor Author

admssa commented Jul 5, 2022

@Andrey9kin Thank you for the fast response.

@Andrey9kin
Copy link
Member

available in 3.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants