Skip to content

Support the Black formatter #1611

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

Merged
merged 8 commits into from
May 5, 2018
Merged

Conversation

brettcannon
Copy link
Member

@brettcannon brettcannon commented May 4, 2018

Fixes #1153

This pull request:

  • Has a title summarizes what is changing
  • Includes a news entry file (remember to thank yourself!)
  • Has unit tests & code coverage is not adversely affected (within reason)
  • Works on all actively maintained versions of Python (e.g. Python 2.7 & the latest Python 3 release)
  • Works on Windows 10, macOS, and Linux (e.g. considered file system case-sensitivity)

Co-authored-by: Josh Smeaton [email protected]

@brettcannon brettcannon requested a review from DonJayamanne May 4, 2018 16:47
return InstallerResponse.Installed;
}
return InstallerResponse.Ignore;
const item = await this.appShell.showErrorMessage(`Formatter ${productName} is not installed. Install?`, 'Yes', 'No');

Choose a reason for hiding this comment

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

This changes the UX, the user has no way to choose a different formatter here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, but you can't install a different linter either when that doesn't exist. I could update it to disable formatting instead of "No" to be more like the linting scenario?

Choose a reason for hiding this comment

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

Yes, but earlier the user was aware of the fact that they had other formatters, with this new UX they won't know. Not until they poke around the docs or settings (majority of the users won't).

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@codecov
Copy link

codecov bot commented May 4, 2018

Codecov Report

Merging #1611 into master will increase coverage by 0.13%.
The diff coverage is 68.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1611      +/-   ##
==========================================
+ Coverage   71.31%   71.44%   +0.13%     
==========================================
  Files         273      274       +1     
  Lines       12700    12723      +23     
  Branches     2282     2285       +3     
==========================================
+ Hits         9057     9090      +33     
+ Misses       3502     3493       -9     
+ Partials      141      140       -1
Impacted Files Coverage Δ
src/client/common/configSettings.ts 88.16% <ø> (ø) ⬆️
src/client/formatters/types.ts 100% <ø> (ø) ⬆️
src/client/providers/formatProvider.ts 94.54% <100%> (+0.31%) ⬆️
src/client/formatters/helper.ts 100% <100%> (ø) ⬆️
src/client/common/types.ts 100% <100%> (ø) ⬆️
src/client/common/installer/productNames.ts 100% <100%> (ø) ⬆️
src/client/formatters/baseFormatter.ts 75.34% <33.33%> (ø) ⬆️
src/client/common/installer/productInstaller.ts 63.38% <47.82%> (+3.07%) ⬆️
src/client/formatters/blackFormatter.ts 83.33% <83.33%> (ø)
...rc/client/debugger/PythonProcessCallbackHandler.ts 52.96% <0%> (-0.66%) ⬇️
... and 4 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 f1e1693...625e350. Read the comment docs.

@ambv ambv mentioned this pull request May 5, 2018
5 tasks
@brettcannon brettcannon merged commit e1dca79 into microsoft:master May 5, 2018
@brettcannon brettcannon deleted the black branch May 5, 2018 04:09
@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
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.

Support for formatting tool "black"
2 participants