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

Format ts code using Prettier #9359

Merged
merged 2 commits into from
Jan 2, 2020
Merged

Conversation

DonJayamanne
Copy link

Part of #2012

  • This PR focuses only on formatting of the typescript code
  • A separate PR will (ready to go) be submitted with the necessary CI changes (simple one line change)
  • Formatting of Python code and the like can be done as a separate PR
  • Files modified:
    • First commit in PR
    • settings.json ensure prettier is used to format code as it is written.
    • tslint and eslint configuration files to ensure prettier respects/uses existing linter settings.
    • package.json new npm packages for prettier
  • Other code files
    • Second commit in PR
    • Changes to format (changes done using prettier, not by hand)

(I'd consider #2012 an epic as there are plenty of proposed changes, format python, js, json, css, Ci changes, etc).

Not submitting individual PRs, as I don't see any real benefit as these are changes made by a tool and not by hand. If required I can submit individual PRs for 100 files or so (that would break it into 11 PRs, thats' large).

Summary:

  • All code formatted according to pretter
  • Code will be formatted automatically in VS Code as you save or manually format.
  • Auto fixing linter issues (where possible by the tools) also taken care of as part for formatting
  • Separate PR will be submitted for (after this is merged)
    • Contributing guide changes
    • Pipelines changes (to fail CI)

@DonJayamanne DonJayamanne requested review from karthiknadig and kimadeline and removed request for kimadeline January 2, 2020 18:08
@DonJayamanne
Copy link
Author

@kimadeline created a new PR as I was having a few problems with the other branch (I had to close rebase and delete the old branch. GitHub lost links, and went looney after that)

@karthiknadig
Copy link
Member

I had discussed this change with @DonJayamanne prior. @DonJayamanne thanks for doing this :)

@codecov-io
Copy link

codecov-io commented Jan 2, 2020

Codecov Report

Merging #9359 into master will increase coverage by 0.35%.
The diff coverage is 57.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9359      +/-   ##
==========================================
+ Coverage   59.97%   60.33%   +0.35%     
==========================================
  Files         545      545              
  Lines       28662    28668       +6     
  Branches     4338     4345       +7     
==========================================
+ Hits        17191    17296     +105     
+ Misses      10535    10443      -92     
+ Partials      936      929       -7
Impacted Files Coverage Δ
src/client/common/application/types.ts 100% <ø> (ø) ⬆️
src/client/debugger/debugAdapter/types.ts 100% <ø> (ø) ⬆️
...atascience/jupyter/jupyterSessionManagerFactory.ts 42.85% <ø> (ø) ⬆️
...ent/debugger/debugAdapter/Common/protocolLogger.ts 7.14% <ø> (ø) ⬆️
src/client/debugger/types.ts 100% <ø> (ø) ⬆️
src/client/datascience/jupyter/jupyterWebSocket.ts 15% <ø> (ø) ⬆️
src/client/common/installer/productPath.ts 100% <ø> (ø) ⬆️
...lient/datascience/jupyter/jupyterSessionManager.ts 8.42% <ø> (ø) ⬆️
src/client/activation/types.ts 100% <ø> (ø) ⬆️
src/client/datascience/data-viewing/types.ts 100% <ø> (ø) ⬆️
... and 370 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43d30fd...57e313b. Read the comment docs.

.eslintrc Show resolved Hide resolved
@DonJayamanne DonJayamanne merged commit 2b6a8f2 into microsoft:master Jan 2, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants