🏗 Report unused private fields #14761
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rsimha
approved these changes
Apr 20, 2018
alabiaga
reviewed
Apr 20, 2018
.eslintrc
Outdated
| @@ -45,6 +45,7 @@ | |||
| "amphtml-internal/no-global": 0, | |||
| "amphtml-internal/no-spread": 2, | |||
| "amphtml-internal/query-selector": 2, | |||
| "amphtml-internal/unused-private-field": 1, | |||
There was a problem hiding this comment.
nitpick, alphabetical order. place below "amphtml-internal/todo-format"
alabiaga
approved these changes
Apr 20, 2018
They don't get stripped in v0.js
jridgewell
force-pushed
the
unused-private-fields
branch
from
16190cb
to
33e6bfd
Compare
This was referenced Apr 20, 2018
jridgewell
added a commit
to jridgewell/amphtml
that referenced
this pull request
Apr 20, 2018
|
This seems to have failed lint on master. |
|
Test flaked, lint isn't the issue. |
|
@jridgewell In that case, we shouldn't be printing all these warnings during push builds, since there's nothing anyone can do about them there, and they distract from real errors. Can you change this line in ... to ... ... and see if it works? (Also do the same with any other warning-only rules you've introduced, until the majority of them are fixed.) |
|
See https://travis-ci.org/ampproject/amphtml/jobs/370169393#L660-L1906 for the output currently being printed on |
|
I'm not sure what format I'm trying to whittle away at the lint warnings. |
This was referenced Apr 23, 2018
This was referenced Apr 26, 2018
jridgewell
added a commit
that referenced
this pull request
Apr 26, 2018
* Cleanup AnimationPlayer's private state Part of #14761. * Nit
This was referenced Apr 26, 2018
peterjosling
added a commit
to emarchiori/amphtml
that referenced
this pull request
Apr 27, 2018
Lots of methods on AmpDoc and Extensions which were marked as private, but being called from exported functions (and potentially being broken by Closure Compiler optimisations). Marked affected methods as public, and removed redundant exported functions (which existed only to expose the private methods). Issue identified in ampproject#14896. Fixes part of ampproject#14761.
peterjosling
added a commit
to emarchiori/amphtml
that referenced
this pull request
Apr 27, 2018
Lots of methods on AmpDoc and Extensions which were marked as private, but being called from exported functions (and potentially being broken by Closure Compiler optimisations). Marked affected methods as public, and removed redundant exported functions (which existed only to expose the private methods). Issue identified in ampproject#14896. Fixes part of ampproject#14761.
peterjosling
added a commit
to emarchiori/amphtml
that referenced
this pull request
Apr 27, 2018
Lots of methods on AmpDoc and Extensions which were marked as private, but being called from exported functions (and potentially being broken by Closure Compiler optimisations). Marked affected methods as public, and removed redundant exported functions (which existed only to expose the private methods). Issue identified in ampproject#14896. Fixes part of ampproject#14761.
glevitzky
pushed a commit
that referenced
this pull request
Apr 27, 2018
* Report unused private fields They don't get stripped in v0.js * ABCDEFG... * Use AST to find the members * Find unused methods * Lint
This was referenced Apr 27, 2018
peterjosling
added a commit
to emarchiori/amphtml
that referenced
this pull request
Apr 30, 2018
Lots of methods on AmpDoc and Extensions which were marked as private, but being called from exported functions (and potentially being broken by Closure Compiler optimisations). Marked affected methods as public, and removed redundant exported functions (which existed only to expose the private methods). Issue identified in ampproject#14896. Fixes part of ampproject#14761.
jridgewell
pushed a commit
that referenced
this pull request
Apr 30, 2018
Lots of methods on AmpDoc and Extensions which were marked as private, but being called from exported functions (and potentially being broken by Closure Compiler optimisations). Marked affected methods as public, and removed redundant exported functions (which existed only to expose the private methods). Issue identified in #14896. Fixes part of #14761.
This was referenced May 3, 2018
jridgewell
added a commit
that referenced
this pull request
May 21, 2018
Just need an `@restricted` annotation Part of #14761.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
They don't get stripped in v0.js
Fixes #14742.