twistedchecker

Merge lp://qastaging/~zomux/twistedchecker/enhance-function-method-class-checking into lp://qastaging/~twistedchecker-dev/twistedchecker/trunk

  1. enhance-function-method-class-checking
  2. Merge into trunk
Proposed by raphael shu
Status: Rejected
Rejected by: raphael shu
Proposed branch: lp://qastaging/~zomux/twistedchecker/enhance-function-method-class-checking
Merge into: lp://qastaging/~twistedchecker-dev/twistedchecker/trunk
Prerequisite: lp://qastaging/~zomux/twistedchecker/check-function-name
Diff against target: 111 lines (+62/-3)
4 files modified
twistedchecker/configuration/pylintrc (+22/-3)
twistedchecker/functionaltests/classname_pass.py (+7/-0)
twistedchecker/functionaltests/functionname_pass.py (+16/-0)
twistedchecker/functionaltests/methodname_pass.py (+17/-0)
To merge this branch: bzr merge lp://qastaging/~zomux/twistedchecker/enhance-function-method-class-checking
Reviewer Review Type Date Requested Status
Jonathan Lange Disapprove
Review via email: mp+116174@code.qastaging.launchpad.net

Description of the change

As there are many special names in Twisted,
the rule should be more flexible to cover these names.

For function/method names:
1. Names like foo_BAR, foo_BAR_BAZ.
   ex. agentc_ADD_IDENTITY, cap_EXPIRE, cmd_NOT_FOUND.
2. Names begin with underscore, as they are special,
   we should not check them.
3. Names with specific patterns.
   ex. dictCode_221_ready, on_suspend_clicked, opt_mime_type.
   These patterns include:
   channel_,check_,dictCode_,dont_,do_,esmtpState_,get_,global_,handle_,
   inotify_,key_,on_,opt_,oscar_,packRequest_,parseRequest_,request_,
   smtpState_,state_,telnet_,testAddingBadProtos_,test_,view_,visitNode_,
   will_,wont_

For class names:
1. Ignore names begin with:
   Record_,SOCKET_,PCP_,PCPII_,HTTP1_,HTTP0_

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

This is interesting.

Clearly, we need some support for special cases. However, there are a few disadvantages to this approach:

 * regexes this long and complicated are almost impossible to maintain
 * just putting the special cases into the regex means that there's no way to figure out *why* they are special
 * some of the names simply represent mistakes in Twisted's code base -- not everything in trunk currently conforms to the coding standard. when those mistakes are fixed, there's nothing to signal that these regexes need to be updated.
 * it makes twistedchecker less useful for other projects that follow Twisted's coding standard (e.g. lp:divmod.org)

What other options do we have?

Definitely in favour of fixing regexes for foo_BAR and foo_BAR_BAZ style names.

jml

review: Disapprove
Revision history for this message
Itamar Turner-Trauring (itamarst) wrote :

One solution is to have a dispatcher object in Twisted for these special cases, instead of manually calling getattr(), and then presumably you could inspect the dispatcher to figure out which method names are valid. This would involve some work on the Twisted side, and then figuring out how this object could expose the information in a way that twistedchecker could get it simply by parsing. What do you think, Jonathan?

Revision history for this message
Jonathan Lange (jml) wrote :

I can't quite see it. What sort of interface would the dispatcher have?

Unmerged revisions

15. By raphael shu

check method/function/class name for twisted

14. By raphael shu

added some special cases

13. By raphael shu

enhance for twisted

12. By raphael shu

added tests for method name checking

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: