-
Notifications
You must be signed in to change notification settings - Fork 132
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
replace all the rules with a map that only contains the rule itself #176
Conversation
Just FYI I didn't actually touch what was in And probably the defrules macro itself can be definitively cleaned up a bit, but let me know if you are happy with this approach first. |
Hey, I'd probably prefer to see the whole change for #175 in one PR, rather than changing all the rules partway through. This is a good start for that though. |
Ah I see, so you mean enumerating all the rules in one go? I just thought it would be better to get this in first because any other change would conflict since I'm basically touching every rule. And by the way should I then not touch the ryles in (comment) right? |
Sorry, I mean I'd rather see one patch set for #175, rather than changing the defrules macro early. I don't think there are any major refactors planned which would cause any merge conflicts with what you're working on.
Yep |
Sorry I don't get how I can enumerate all the rules without actually changing the The defrules macro stops working if I change the structure of the rules in any way right? And by the way I had another thought, since we are talking about restructuring the rules, it would be probably even better to make it even more explicit. So instead of this:
we could have this:
Specially for some long rules this would make it even easier to understand. |
Hey, what I'm saying is, I don't want to merge a partial PR towards #175, when you provide a PR, provide all of it in one patch, i.e. the changes to the rules syntax, along with the code to allow enumerating rules. |
601057d
to
5f44550
Compare
5f44550
to
51c4083
Compare
Sorry for the delay but now finally I work with Clojure and use Kibit so I would definitively like to get this forward.. However there are 88 rules and I haven't really written any of them, so would like to know what is needed for this PR to be merged. The Or the changes about enumerating should be in |
Closing this PR in light of kibit entering maintenance status. (See this comment for further details.) |
With this simple mechanical change (done simply with the power of Emacs query-replace-regexp and a few manual tweaks), all the tests are still passing, and it's a starting point to extend the rules to have a more rich structure, as discussed in
#175