lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Prescott Nasser <geobmx...@hotmail.com>
Subject RE: Outstanding issues for 3.0.3
Date Thu, 02 Aug 2012 16:13:12 GMT
I don't think we ever fully adopted the style guidelines, probably not a terrible discussion
to have. As for this release, I think that by lazy consensus we should branch the trunk at
the end of this weekend (say monday), and begin the process of cutting a release. - my $.02
below


> 1) Usage of "this" prefix when not required.
>
> this.blah = blah; <- required this.
> this.aBlah = blah; <- optional this, which Re# doesn't like.
>
> I'm assuming consistency wins here, and 'this.' stays, but wanted to double check.

I'd error with consistency


>
> 2) Using different conventions for fields and parameters\local vars.
>
> blah vs. _blah
>

> Combined with 1, Re# wants (and I'm personally accustomed to):
>
> _blah = blah;
>


For private variables _ is ok, for anything else, don't use _ as it's not CLR compliant


> However, that seems to violate the adopted style.
>
> 3) Full qualification of type names.
>
> Re # wants to remove redundant namespace qualifiers. Leave them or remove them?
>

I try to remove them

> 4) Removing unreferenced classes.
>
> Should I remove non-public unreferenced classes? The ones I've come across so far are
private.
>

I'm not sure I understand - are you saying we have classes that are never used in random places?
If so, I think before removing them we should have a conversation; what are they, why are
they there, etc. - I'm hoping there aren't too many of these..

> 5) var vs. explicit
>
> I know this has been brought up before, but not sure of the final disposition. FWIW,
I prefer var.
>

I use var with it's plainly obvious the object var obj = new MyClass(). I usually use explicit
when it's an object returned from some function that makes it unclear what the return value
is:


var items = search.GetResults();

vs

IList<SearchResult> items = search.GetResults(); //prefer


>
> There are some non-Re# issues I came across as well that look like artifacts of code
generation:
>
> 6) Weird param names.
>
> Param1 vs. directory
>
> I assume it's okay to replace 'Param1' with something a descriptive name like 'directory'.
>

Weird - I think a rename is OK for this release (Since we're ticking up a full version number),
but I believe changing param names can potentially break code. That said, I don't really think
we need to change the names and push the 3.0.3 release out, and if it does in fact cause breaking
changes, I'd be a little careful about how we do it going forward to 3.6.

> 7) Field names that follow local variable naming conventions.
>
> Lots of issues related to private vars with names like i, j, k, etc. It feels like the
right thing to do is to change the scope so that they go back to being local vars instead
of fields. However, this requires a much more significant refactoring, and I didn't want to
assume it was okay to do that.
>

I'd avoid this for now - a lot of this is a carry over from the java version and to rename
all those, it starts to get a bit confusing if we have to compare java to C# and these are
all changed around.



> If these questions have already been answered elsewhere and I missed the documentation/FAQ/developer
guide, then I apologize and would appreciate the links. Alternatively, if someone has a Re#
rule config that they are willing to post somewhere, I would be glad to use it.
>

I think we talked about Re#'s rules at one point, I'll try to dig that conversation up and
see where it landed. It's probably a good idea for us to build rules though.

> - Zack
>
>
> On Jul 27, 2012, at 12:00 PM, Itamar Syn-Hershko wrote:
>
> > The cleanup consists mainly of going file by file with ReSharper and trying
> > to get them as green as possible. Making a lot of fields readonly, removing
> > unused vars and stuff like that. There are still loads of files left.
> >
> > I was also hoping to get to updating the spatial module with some recent
> > updates, and to also support polygon searches. But that may take a bit more
> > time, so it's really up to you guys (or we can open a vote for it).
> 		 	   		  
Mime
View raw message