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

Remove jQuery #1835

Closed
wants to merge 34 commits into from
Closed

Conversation

kx-chen
Copy link
Contributor

@kx-chen kx-chen commented Jun 11, 2020

Closes #1807

@ThiefMaster
Copy link
Member

What's wrong with const? Unless we reassign I think const makes perfect sense.

@davidism
Copy link
Member

davidism commented Jun 11, 2020

I agree, use const if the value is not expected to change.

Do in general look for old patterns though, we probably shouldn't use var regardless.

src/werkzeug/debug/shared/debugger.js Outdated Show resolved Hide resolved
src/werkzeug/debug/shared/debugger.js Outdated Show resolved Hide resolved
src/werkzeug/debug/shared/debugger.js Outdated Show resolved Hide resolved
src/werkzeug/debug/shared/debugger.js Outdated Show resolved Hide resolved
@chrisnguyn
Copy link
Contributor

Ready for maintainer review.

@obfusk
Copy link
Contributor

obfusk commented Jun 12, 2020

tbtools.py still includes a <script> tag for jquery.js.

@ThiefMaster
Copy link
Member

I would reformat it with prettier. Here's a decent config (but can be changed of course in case you prefer double quotes to be consistent with the Python style):

printWidth: 100
tabWidth: 2
useTabs: false
semi: true
singleQuote: true
quoteProps: 'consistent'
trailingComma: 'es5'
bracketSpacing: false
jsxBracketSameLine: false
arrowParens: 'avoid'
endOfLine: 'lf'

I'd also suggest running eslint with the recommended preset on the file and fix stuff it finds (many things may be autofixable).

src/werkzeug/debug/shared/debugger.js Outdated Show resolved Hide resolved
src/werkzeug/debug/shared/debugger.js Outdated Show resolved Hide resolved
src/werkzeug/debug/shared/style.css Outdated Show resolved Hide resolved
@kx-chen

This comment has been minimized.

@davidism davidism added this to the 2.0.0 milestone Jun 19, 2020
@davidism davidism mentioned this pull request Jun 19, 2020
@davidism
Copy link
Member

Couldn't push to this branch, so squashed and rebased in #1857.

@davidism davidism closed this Jun 19, 2020
@davidism davidism deleted the 1807-remove-jquery branch June 19, 2020 21:59
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 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.

Remove jQuery
7 participants