Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Enforce "noUnusedLocals" : Report errors on unused locals. #200

Merged

Conversation

mayurkale22
Copy link
Member

Add --noUnusedLocals Compiler Options to disallows unused imports, variables, functions and
private class members.

Copy link
Contributor

@justindsmith justindsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@mayurkale22 mayurkale22 merged commit 9158af5 into census-instrumentation:master Nov 26, 2018
@mayurkale22 mayurkale22 deleted the noUnusedLocals branch November 26, 2018 19:13
@@ -64,6 +63,7 @@ export class BaseView implements View {
/** true if the view was registered */
registered = false;
/** An object to log information to */
// @ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we disabling if we want to avoid redundant definitions?

@@ -30,6 +29,7 @@ export abstract class SpanBase implements types.Span {
/** Indicates if this span was ended */
private endedLocal = false;
/** Indicates if this span was forced to end */
// @ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we disabling if we want to avoid redundant definitions?

@@ -40,6 +38,7 @@ export class CoreTracer implements types.Tracer {
/** A list of end span event listeners */
private eventListenersLocal: types.SpanEventListener[] = [];
/** A list of ended root spans */
// @ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we disabling if we want to avoid redundant definitions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, I added to suppress errors. I am not completely aware of CoreTracer properties, hence didnt remove that variable. Anyways, I will remove it in next PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, I added to suppress errors. I am not completely aware of CoreTracer properties, hence didnt remove that variable. Anyways, I will remove it in next PR.

How users will use Typescript for their projects in this case?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like each case it's a private unused variable, so we should clean them up. Let's just remove them in the next PR like @mayurkale22 suggested. They aren't api breaking to remove since they're private.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants