-
Notifications
You must be signed in to change notification settings - Fork 12
Consolidate let-binding rules into let-replacement directory #642
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
Consolidate let-binding rules into let-replacement directory #642
Conversation
Co-authored-by: jackfirth <[email protected]>
jackfirth
left a comment
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.
@copilot There's a few other rules that need to move here which are currently parts of other refactoring suites. Those rules should be extracted into new suites and files into the new directory. The only two rules like this I'm aware of are:
cond-let-to-cond-definematch-let-to-match-define
Both the implementations and tests of those rules should be moved.
Co-authored-by: jackfirth <[email protected]>
Done. Both rules and their tests have been extracted and moved to the let-replacement directory:
The original rules were removed from |
jackfirth
left a comment
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.
@copilot This is almost ready. However, there's one issue: not everything in let-binding-suggestions should be moved into this directory. Only the rules that rewrite let forms into define forms should be moved. That includes:
let-to-definedefine-let-to-double-definebegin0-let-to-define-begin0
These rules should be moved to a let-replacement.rkt file in the new directory, and the other rules should remain in let-binding-suggestions.rkt. That way, the only rules in the let-replacement directory are rules for replacing let and let-like forms with define.
…ement.rkt Co-authored-by: jackfirth <[email protected]>
Done. The let-binding-suggestions suite has been split:
Tests have been updated accordingly. All 667 tests pass. (8a5752e) |
Consolidates let-to-define refactoring rules and their test files into
default-recommendations/let-replacement/, mirroring the existingloops/directory structure.Files moved:
let-binding-suggestions.rktand 4 test files →let-replacement/private/let-binding.rkt→let-replacement/private/Files split:
let-binding-suggestions.rktwas split into:let-replacement.rkt- contains only rules that convertlettodefine:let-to-define,define-let-to-double-define,begin0-let-to-define-begin0let-binding-suggestions.rkt- contains otherlet-related rules:named-let-to-plain-let,let-values-then-call-to-call-with-values,delete-redundant-letRules extracted and moved:
cond-let-to-cond-definefromconditional-shortcuts→let-replacement/cond-let-replacement.rktmatch-let-to-match-definefrommatch-shortcuts→let-replacement/match-let-replacement.rktNew files created:
cond-let-replacement.rktandcond-let-replacement-test.rktmatch-let-replacement.rktandmatch-let-replacement-test.rktlet-replacement.rktandlet-replacement-test.rktImport paths updated in:
default-recommendations.rktconditional-shortcuts.rktmatch-shortcuts.rktloops/for-loop-shortcuts.rktloops/named-let-loopification.rktloops/private/syntax-classes.rktAll rules in the
let-replacementdirectory now exclusively handle convertinglet-style bindings todefine-style bindings. Otherlet-related rules remain inlet-binding-suggestions.rktfor better organization and maintainability.let-to-definesuggestions #641Original prompt
let-to-definesuggestions #641💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.