-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
buildGlobalParametersMap implementation & testing #5399
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@@ -28,4 +28,4 @@ export async function createHardhatRuntimeEnvironment( | |||
} | |||
|
|||
export { resolvePluginList } from "./internal/plugins/resolve-plugin-list.js"; | |||
export { buildGlobalParameterMap } from "./internal/global-parameters.js"; | |||
export { buildGlobalParametersMap } from "./internal/global-parameters.js"; |
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.
@schaable We already discussed changing this name in a future PR, but just FYI, this was correct. It can be tricky for us, native Spanish speakers, but it's ParameterMap, not ParametsMap. You have to keep the first word in singular, even though the name represents a collection of those things — pretty much the opposite of what you'd do in Spanish.
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.
LGTM, but let's not forget about the naming issue
I've added an issue to track it. |
Part of #5379
This should be easy to review by going through it commit by commit.
Merge this after #5390