Code

Opened 14 months ago

Last modified 10 months ago

#20436 assigned Cleanup/optimization

Cleanup/Optimization of javascript code with JSLint in admin

Reported by: galuszkak Owned by: galuszkak
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: jgeskens, Amizya Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Django admin is using javascript for many controls. It will be great to improve this code using javascript linter like JSLint.

Also this is nice start to have some little cleanups in JS.

Attachments (0)

Change History (11)

comment:1 Changed 14 months ago by jgasteiz

  • Cc javi.manzano.oller@… added
  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to jgasteiz
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 14 months ago by jgasteiz

  • Cc javi.manzano.oller@… removed
  • Owner jgasteiz deleted
  • Status changed from assigned to new
  • Triage Stage changed from Accepted to Unreviewed

comment:3 Changed 14 months ago by jgeskens

  • Cc jgeskens added
  • Triage Stage changed from Unreviewed to Accepted

That would be a great idea. Let's do it!

comment:4 Changed 14 months ago by galuszkak

  • Owner set to galuszkak
  • Status changed from new to assigned

comment:5 Changed 11 months ago by Amizya

  • Cc Amizya added

comment:6 Changed 11 months ago by timo

I'm not sure this should be accepted. Note the following from our contributing docs:

Note, however, that patches which only remove whitespace (or only make changes for nominal PEP 8 conformance) are likely to be rejected, since they only introduce noise rather than code improvement. Tidy up when you’re next changing code in the area."

comment:7 Changed 11 months ago by anonymous

@timo

To address your concerns. It's not only removing whitespaces. My changes are doing some cleanup refactorings regarding also good patterns in JavaScript. Also I want to migrate all code to use ECMA5 using "use strict". This involves backward compatibility test. In the end I'm working on making code more OOP and to think how to solve problem of lacking of JavaScript unit tests. We definitely should do something about that.

For know there is sometimes functional code and sometimes object oriented. (and we pollute global namespace which is very bad JS pattern.)

So this is not ticket about changing whitespaces. It's about improving code performance and introduce more best practices. It's challenging and I'm try to that in my free time. Hopefully do eomt pull request in near future.

Cheers,
Kamil

comment:8 Changed 11 months ago by galuszkak

This post above is mine. Soory I forgot to log in.

Cheers,
Kamil

comment:9 Changed 11 months ago by aaugustin

Tim, I'm quite sure the admin JS needs to migrate to modern coding standards. The design is too far from the best practices of 2013. Let's leave this ticket as "accepted".

comment:10 Changed 11 months ago by julien

I agree that some refactorings would be welcome. De-cluttering the global namespace in particular sounds very appealing.

For a patch to be accepted there would need to be regression tests added. We have a system in place for running integration tests with Selenium, but this could also be a good opportunity to start working on a solution for unit-testing.

My only concern is that the scope feels a little large for just one ticket. If possible I think it'd be easier to manage by breaking this down in chunks. See for example an existing ticket for the filtered widget: #15220.

comment:11 Changed 10 months ago by timo

  • Easy pickings unset

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as assigned
The owner will be changed from galuszkak to anonymous. Next status will be 'assigned'
The ticket will be disowned. Next status will be 'new'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.