Use ESlint instead of TSlint #7719
Conversation
|
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR Further commands available:
|
|
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR Further commands available:
|
|
/run-acceptance-tests |
|
Please view the results of the PR Build + Acceptance Tests Run Here |
| @@ -32,7 +32,7 @@ const grpcChannelOptions = { "grpc.max_receive_message_length": maxRPCMessageSiz | |||
| /** | |||
| * excessiveDebugOutput enables, well, pretty excessive debug output pertaining to resources and properties. | |||
| */ | |||
| export let excessiveDebugOutput: boolean = false; | |||
| export const excessiveDebugOutput: boolean = false; | |||
There was a problem hiding this comment.
This might be substantive?
`const` is a signal that the identifier won't be reassigned. `let` is a signal that the variable may be reassigned,
Will this prevent a user from reassigning this value?
There was a problem hiding this comment.
Good catch.
I think the original intention was to allow it to be assigned. Though, I suspect nobody is setting it these days (besides maybe a Pulumi engineer on a rare occasion).
With ES6 modules, you aren't allowed to reassign an exported value from outside the module (imports are a read-only live view of the value), so it might be worth considering changing it to const to discourage anyone setting it from outside the module going forward.
Changing it to const does mean it isn't useful unless you're modifying the source directly (either by modifying a copy of @pulumi/pulumi inside node_modules or using yarn link locally). If being able to set it is missed, we could consider adding a setExcessiveDebugOutput function, or perhaps better, switch to using an environment variable that could work across all SDKs.
There was a problem hiding this comment.
In that case let's accept this change for now.
There was a problem hiding this comment.
If being able to set it is missed, we could consider adding a setExcessiveDebugOutput function, or perhaps better, switch to using an environment variable that could work across all SDKs.
Could we open an issue for it? I could also work on this issue as well as a follow up.
|
This is looking very helpful 👍 Almost all changes appear to be strictly lint-level (not substantive) except one change from Also I've not used these tools in a long time so can't comment on the tslint config, but I'd vote to check this in and continue iterating as needed. |
|
The |
| @@ -32,7 +32,7 @@ const grpcChannelOptions = { "grpc.max_receive_message_length": maxRPCMessageSiz | |||
| /** | |||
| * excessiveDebugOutput enables, well, pretty excessive debug output pertaining to resources and properties. | |||
| */ | |||
| export let excessiveDebugOutput: boolean = false; | |||
| export const excessiveDebugOutput: boolean = false; | |||
There was a problem hiding this comment.
Good catch.
I think the original intention was to allow it to be assigned. Though, I suspect nobody is setting it these days (besides maybe a Pulumi engineer on a rare occasion).
With ES6 modules, you aren't allowed to reassign an exported value from outside the module (imports are a read-only live view of the value), so it might be worth considering changing it to const to discourage anyone setting it from outside the module going forward.
Changing it to const does mean it isn't useful unless you're modifying the source directly (either by modifying a copy of @pulumi/pulumi inside node_modules or using yarn link locally). If being able to set it is missed, we could consider adding a setExcessiveDebugOutput function, or perhaps better, switch to using an environment variable that could work across all SDKs.
|
PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR Further commands available:
|
|
/run-acceptance-tests |
|
Please view the results of the PR Build + Acceptance Tests Run Here |
Description
Migrated TSlint configs to ESlint ones using tslint-to-eslint-config tool, and refined the configs to better match the current coding style.
Changes:
semicolonFixes #4698 (Start linting all TypeScript code with ESLint)
Checklist