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

Linting #27

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Linting #27

wants to merge 3 commits into from

Conversation

Iain-S
Copy link

@Iain-S Iain-S commented Aug 19, 2020

I ran ESLint demo on the JS:

  • There are other things we could do to it if we wanted to move to the ES6 standard (e.g. use let or const in place of var)

I ran flake8 on the .py files:

  • There are a number of unused variables in lab_black.py, which we should probably use or delete
  • Other than the unused variables, there are no flake8 errors
  • I created a black_formatter variable in nb_black because it doesn't need to be shared with lab_black and it was causing an unused variable error

IainS QMUL added 2 commits August 19, 2020 11:57
- Give a function name for easier debugging
- Use === comparison instead of == to avoid type coercion
- Set a black-friendly max line length of 88 chars in .flake8
- Note that there are a number of unused variables in lab_black.py
@@ -1,7 +1,9 @@
# -*- coding: utf-8 -*-
from __future__ import absolute_import, division, print_function, unicode_literals

from lab_black import BlackFormatter, black_formatter, unload_ipython_extension
Copy link
Owner

Choose a reason for hiding this comment

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

If you remove unload_ipython_extension, I guess notebook users are not able to unload the extension

Copy link
Author

Choose a reason for hiding this comment

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

Ah, true but I'm not sure unloading works as it should anyway.

If you clone my reload branch and run the Reload.ipynb, it looks as though format_cell() gets called once for each time you have loaded the extension and cells still get formatted after calling %unload_ext.

We could have a separate unload function in nb_black.py (which is what I have suggested in the latest commit in this branch).

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

Successfully merging this pull request may close these issues.

2 participants