2 Replies Latest reply on Dec 10, 2010 10:31 AM by tcunning

    Checkstyle

    tcunning

      I've taken a look at what's left to clear up checkstyle.     Our errors boil down to a few categories - I don't mind going through and fixing them if that's what we want, but if not, it'd probably be helpful to comment out the rules to clear out the noise :

       

      Checkstyle rule "DesignForExtension" :

       

      Code example :

      Service Registration.java,

       

      ServiceRegistration(QName serviceName,
                  Endpoint endpoint,
                  HandlerChain handlers,
                  ServiceRegistry registry,
                  ServiceDomain domain) {
      
              _serviceName = serviceName;
              _endpoint = endpoint;
              _handlers = handlers;
              _registry = registry;
              _domain = domain;
          }
      

       

      all of the parameters get the error that they should be final.     We discussed this rule on a call and I don't believe we came to any conclusion about what to do about it.

       

      Checkstyle rule "FinalLocalVariable" :
      Code example :

      ServiceRegistration.java

       

      public ServiceDomain getDomain() {
              return _domain;
          }
      
          @Override
          public QName getName() {
              return _serviceName;
          }
      

       

      both methods get the error that they are not designed for extension and that they need to be abstract, final or empty.      I believe we discussed this rule on one of the calls and decided to scrap it but I wanted to double check before submitting a pull request with it.

        • 1. Re: Checkstyle
          mageshbk

          Tom, I think you mismatched the rule names with the sample.

          Tom Cunningham wrote:

          Checkstyle rule "DesignForExtension" :

           

          This is unnecessary and should be removed from the rules.

          Tom Cunningham wrote:

          Checkstyle rule "FinalLocalVariable"

           

           

          This is a safety measure and I do not mind having it. As long as you have no intention of modifying the parameters, it should be final.

          • 2. Re: Checkstyle
            tcunning

            You're right - I have the rule names mixed up.     

             

            As for FinalLocalVariable, there's two rules that check this same area.

             

            FinalLocalVariable : http://checkstyle.sourceforge.net/config_coding.html#FinalLocalVariable

            ParameterAssignment : http://checkstyle.sourceforge.net/config_coding.html#ParameterAssignment

             

            I think having ParameterAssignment enabled and FinalLocalVariable disabled would cover us without being so draconian.       If we really want to make all parameters final, that's fine - we can leave FinalLocalVariable enabled, but I think we're already covered for safety by the ParameterAssignment rule.