Re: [Clirr-devel] Patch: increase flexibility of scope selection

[<lak...@t-...>]

Hi Simon,

in general ScopeSelector looks like a good idea to clean up the 
implementation. I don't know whether public/protected is also hardwired 
in the logic anywhere, so we should do some testing first before we make 
it public in the Ant task.

Two minor points:

    * MethodSetCheck.getMethodId() is supposed to return the method
      sigmature as it appears in the code / Javadoc. With the
      ScopeSelector changes, it returns an extra "package" for package
      visible methods.
    * In most calls to ScopeSelector the flags are determined by the
      calling code only to pass them to the called ScopeSelector method.
      Could the code be simplified further by passing the AccessFlags
      object (JavaClass, Method, Field) directly?

If you can update the tests and fix getMethodId(), I think we can commit 
ScopeSelector.

Thanks,
Lars

PS: I think your encoding problems are related to your editor settings. 
In the license header of ScopeSelector there is a question mark instead 
of the umlaut character...


Simon Kitching wrote:

>Hi,
>
>Well, here's my first patch proposal for your consideration.
>
>When generating reports, users may want to see differences in package or
>even private-scope objects. Currently, clirr hardwires "public|private"
>in a number of places. This patch is intended to introduce a more
>flexible way of managing what objects get selected.
>
>The unit tests don't currently pass. I believe this is just because the
>"report" message strings are now a little different. I will try to
>figure out the easiest way to update these in the unit tests soon. In
>the meantime, any feedback on this code would be appreciated.
>
>The ScopeSelector.java file belongs in the "event" package.
>
>I do need to write some unit tests for this code, but wanted to get some
>feedback on the initial code first.
>
>NB: the diffs also report the line with "Lars Kuhne" has changed in this
>file. This is some side-effect of the fact that the u-umlaut cannot be
>represented in ascii. It's either CVS or jedit that's having problems
>with this char, probably also related to my system language setting. 
>
>If you like this patch, then some code should probably be written to
>allow ANT options to select the scope.
>
>My next patch proposal will provide a way to run tests from the
>command-line, rather than needing to be invoked from Ant, and will
>provide a way to set the selected scope. It's almost ready.
>
>Cheers,
>
>Simon
>
>  
>