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

chore: simplify moduleContexttoProto #1329

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

alecthomas
Copy link
Collaborator

It's preferable to pass only exactly what's needed into a function. In this case it was receiving a full slice of modules, and a module name, and only allowing a single entry.

@alecthomas alecthomas requested a review from a team as a code owner April 24, 2024 22:10
@alecthomas alecthomas requested review from deniseli and removed request for a team April 24, 2024 22:10
@alecthomas alecthomas enabled auto-merge (squash) April 24, 2024 22:10
@alecthomas alecthomas mentioned this pull request Apr 24, 2024
It's preferable to pass only exactly what's needed into a function. In
this case it was receiving a full slice of modules, and a module name,
and only allowing a single entry.
@alecthomas alecthomas force-pushed the aat/simplify-modulecontextoproto branch from f770f24 to 6792ee2 Compare April 24, 2024 22:12
@alecthomas alecthomas merged commit 3deb948 into main Apr 24, 2024
11 checks passed
@alecthomas alecthomas deleted the aat/simplify-modulecontextoproto branch April 24, 2024 22:16
@deniseli deniseli added the approved Marks an already closed PR as approved label Apr 25, 2024
@deniseli
Copy link
Contributor

sweet, this was a lot easier to follow than the original logic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Marks an already closed PR as approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants