Add exported function to remove rules#316
Conversation
|
Hi! Thanks for this work. I think adding a "RemoveRule" is great, but I went ahead and merged #320 because I do believe there is a need to replace the rules completely occasionally (non-parallel tests, for instance). However, that caused merge conflicts with your PR here. |
Hi Steve, just dropping by to say Ive seen your comment and will fix the conflicts in the PR. I currently don’t have access to a laptop for a few weeks, but will get to it once I do. |
|
@StevenACoffman all good to be merged now! |
|
Thanks very much for your patience and contribution! I really like where we ended up with the functionality. |
|
Thank you! I was looking to this feature as well! |
Hey there!
I'm using gqlparser in another project. There, I add a number of custom rules on top of the existing ones.
I'm having an issue with writing tests though. Since the
rulesglobal variable is not exported, and there exists noRemoveRulefunctionality, my tests start to interfere with each other as this global state is never reset between test cases.For that reason, I'd like to add this MR, so I can clear this state between runs.
Things I've considered:
rulesglobal variable. Decided against that as that would be a much more impactful changeinit()functions, to something else so that they can be re-initialized.RemoveRulethat does a string-based match on the rule name, and remove it (actually filter it out on copy) from the list. Then re-assign the still unexported field to this new, filtered, list.Adding meaningful tests for this was hard as the tests for the validator are not in the same package (
validatorvsvalidator_test). I added tests to assure there are no errors when running theRemoveRulefunction both when there is a match on a rule, and when there is not.If a different approach is preferred I'm happy to alter the MR. I thought I'd start small and get your thoughts on the right strategy before trying to rewrite too much.
I have: