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

No unused vars part of ts #4670

Merged
merged 3 commits into from
Apr 25, 2018

Conversation

cliffkoh
Copy link
Contributor

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ npm run change

Description of changes

Now, unused vars issues show up in an IDE like VSCode :) Also, tslint is notably faster (28% faster! 17.897s -> 13s).

Yay for faster build times, more reliable code cleanliness, and smaller bundle sizes from all the deleted, redundant code.

@cliffkoh
Copy link
Contributor Author

As a bonus, you can now do this (with cmd+shift+b or ctrl+shift+b)

inline build

this.setState({ themeRules: themeRules }, this._makeNewTheme);
}
}
*/}
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for keeping this? (It looks like _cognitiveVisionCallback was removed in the PR but is still called in the commented out code.)

@cliffkoh cliffkoh merged commit ea79638 into microsoft:master Apr 25, 2018
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request Apr 26, 2018
* master: (42 commits)
  Applying package updates.
  ProgressIndicator: Finish conversion to mergeStyles (microsoft#4595)
  Fix props validation for Breadcrumb (microsoft#4666)
  No unused vars part of ts (microsoft#4670)
  Picker/Autofill: fixes several minor bugs. (microsoft#4569)
  Fix Calendar component PREV/NEXT month, year, and "Go to today" handlers firing twice (microsoft#4662)
  Applying package updates.
  Merge styles order (microsoft#4664)
  Fabric component: revert class change and make it backwards compatible (microsoft#4671)
  Addressing Issue microsoft#3707 - OverflowSet: Add the ability to set aria-label (microsoft#4667)
  Fix input type for Tile ARIA label prop (microsoft#4668)
  Fix theme slots for DetailsList header colors (microsoft#4658)
  Applying package updates.
  Jolore/calendar updates (microsoft#4643)
  Remove wordWrap setting. (microsoft#4657)
  Pivot: convert to mergeStyles - part 1  (microsoft#4656)
  Use the `data-is-scrollable` attribute on the correct ScrollablePane div (microsoft#4602)
  Applying package updates.
  Remove unused iconClassName prop from Nav.types (microsoft#4634)
  Jest snapshots: classes in animations should autoexpand. (microsoft#4647)
  ...
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request Apr 26, 2018
* master: (34 commits)
  Applying package updates.
  ProgressIndicator: Finish conversion to mergeStyles (microsoft#4595)
  Fix props validation for Breadcrumb (microsoft#4666)
  No unused vars part of ts (microsoft#4670)
  Picker/Autofill: fixes several minor bugs. (microsoft#4569)
  Fix Calendar component PREV/NEXT month, year, and "Go to today" handlers firing twice (microsoft#4662)
  Applying package updates.
  Merge styles order (microsoft#4664)
  Fabric component: revert class change and make it backwards compatible (microsoft#4671)
  Addressing Issue microsoft#3707 - OverflowSet: Add the ability to set aria-label (microsoft#4667)
  Fix input type for Tile ARIA label prop (microsoft#4668)
  Fix theme slots for DetailsList header colors (microsoft#4658)
  Applying package updates.
  Jolore/calendar updates (microsoft#4643)
  Remove wordWrap setting. (microsoft#4657)
  Pivot: convert to mergeStyles - part 1  (microsoft#4656)
  Use the `data-is-scrollable` attribute on the correct ScrollablePane div (microsoft#4602)
  Applying package updates.
  Remove unused iconClassName prop from Nav.types (microsoft#4634)
  Jest snapshots: classes in animations should autoexpand. (microsoft#4647)
  ...
@cliffkoh cliffkoh deleted the no_unused_vars_part_of_ts branch October 2, 2018 17:44
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019
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.

3 participants