Re: [hibernate-dev] Re: Hibernate Search: approval for changes, commit conventions?
Sanne Grinovero wrote: I knew about the requirement to change the UID, but really hadn't a clue about the default behaviour: I thought it was unspecified and I really dislike that word. The default behaviour is semi-specified - specifically, it's based on a SHA1 hash of: class name and modifiers implemented interface names field names, types and modifiers (excluding 'private static' and 'private transient' fields) presence of a static initializer constructor signatures and modifiers (excluding private constructors) method names, signatures and modifiers (excluding private methods) The unspecified bit creeps in because it's not specified precisely what synthetic members and classes used to implement language features like class literals and inner classes should be named, so different compiler implementations are free to make different choices, giving different generated UIDs. Max. signature.asc Description: OpenPGP digital signature ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
[hibernate-dev] Re: Hibernate Search: approval for changes, commit conventions?
Hi, I'll cut away the questions I don't need to ask more about. 2008/5/29 Emmanuel Bernard [EMAIL PROTECTED]: Hi Sanne On May 28, 2008, at 14:01, Sanne Grinovero wrote: [...] 1)is it ok in case of a code revision to have a commit of 100 files for very trivial changes ( wrong formatting, spelling typos in comments, unused import removals); do I have to warn you before commits of this type? should I leave them all as-is? If you do this massive work, you need to warn people in advance and do it in a day. It essentially means people will have hard time to sync up their current work while you do that. If you cna't then, do it by small batches. Yes I understand that, but no code changes are involved just some comments, imports and final changes so I hope not to cause difficult conflicts, if any. How should I proceed? My changes are ready for commit, I just wait for everybody saying it's ok, so I can update first. I've seen Hardy committed this morning; Also if my working this way goes in the way of other developers, I'll just revert them all. 5)static logger: some classes (e.g. org.hibernate.search.engine.QueryLoader) forget to declare the Logger as static, if this is intentional I would like to know more about it. May I fix them all if I find more? Is it because of hot-redeploy? In this case I would need to remove some statics;-) http://www.slf4j.org/faq.html#declared_static But for classes used a lot, I tend to keep the static modifier. Nice I didn't now that; I changed the logger I used for configuration parsing to non-static, looks much better. Also I think DocumentBuilder could benefit from a non-static logger, but it is (also) being used by one static method for a trace; I could a) leave it static b) have the method get it's own logger c) remove the tracing d) convert all using methods from static I would prefer c), or d) but this may create a lot of changes. A)defensive copy on public API (FindBugs): FindBugs is complaining about May expose internal representation by incorporating reference to mutable object in serveral places. I suppose we can trust our own code, but for example in FullTextQuery setProjection(String... fields) it would be wise to make an own private copy of the data. Too much paranoia? Also it would have some performance cost. What do you want to copy? Projection is essentially a copy already (unless you reference THIS but in this case you want the mutable object). Otherwise, the method parameters are immutable, so this is not a problem. I guess this was not a very good example;) FindBugs is speaking about the Array, a fool user may initialize the arguments as an array, give it to the method, keeping a reference to the array, to later (concurrently?) change the references to other objects or change positions, so having access to Searche's internal state. My questions is a bit more general, not specifically about this method but about us ignoring this FindBugs warning, be aware of it; I suppose it would be too much paranoia to really change all the dangerous methods, just verifying it is an issue you are aware of; rock solid libraries shouldn't allow it, but it has performance impact. B) there's a STATIC field in SharedReaderProvider: static Field subReadersField it is looking quite suspect, is that really meant to be static? Yes it's on purpose, it's a pointer to a Field object, no need to pay the lookup more than once. Ah sorry I should have looked better, I thought a Lucene Field, not reflection. May I make it final, to enhance readability and to avoid null checks in code? (initializing it with a static helper method) C)Workspace contains many maps to needed objects, all having the same key DirectoryProvider; also locks are mantained in an array; Woudn't it be easier to have a single map, returning a container object with all needed stuff; This object could be passed to all other methods instead of the dir.Provider and avoid many more gets on the maps. also some locking code could go local in this container class, I've some difficulty in tracking the lock sequences. Go ahead (open a JIRA issue), make sure it does no slow things down - it shouldn't from what I've seen) You mean slow down of development time or Search's performance? Sanne ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
[hibernate-dev] Re: Hibernate Search: approval for changes, commit conventions?
On May 29, 2008, at 08:59, Sanne Grinovero wrote: Hi, I'll cut away the questions I don't need to ask more about. 2008/5/29 Emmanuel Bernard [EMAIL PROTECTED]: Hi Sanne On May 28, 2008, at 14:01, Sanne Grinovero wrote: [...] 1)is it ok in case of a code revision to have a commit of 100 files for very trivial changes ( wrong formatting, spelling typos in comments, unused import removals); do I have to warn you before commits of this type? should I leave them all as-is? If you do this massive work, you need to warn people in advance and do it in a day. It essentially means people will have hard time to sync up their current work while you do that. If you cna't then, do it by small batches. Yes I understand that, but no code changes are involved just some comments, imports and final changes so I hope not to cause difficult conflicts, if any. How should I proceed? My changes are ready for commit, I just wait for everybody saying it's ok, so I can update first. I've seen Hardy committed this morning; Also if my working this way goes in the way of other developers, I'll just revert them all. Go ahead 5)static logger: some classes (e.g. org.hibernate.search.engine.QueryLoader) forget to declare the Logger as static, if this is intentional I would like to know more about it. May I fix them all if I find more? Is it because of hot-redeploy? In this case I would need to remove some statics;-) http://www.slf4j.org/faq.html#declared_static But for classes used a lot, I tend to keep the static modifier. Nice I didn't now that; I changed the logger I used for configuration parsing to non-static, looks much better. Also I think DocumentBuilder could benefit from a non-static logger, but it is (also) being used by one static method for a trace; I could a) leave it static b) have the method get it's own logger c) remove the tracing d) convert all using methods from static I would prefer c), or d) but this may create a lot of changes. You can't do (d) really as one of the static method is used outside, you cannot do (c). We could move static methods to a different class but (a) is easier for now. This is not like it would greatly enhance the software :) A)defensive copy on public API (FindBugs): FindBugs is complaining about May expose internal representation by incorporating reference to mutable object in serveral places. I suppose we can trust our own code, but for example in FullTextQuery setProjection(String... fields) it would be wise to make an own private copy of the data. Too much paranoia? Also it would have some performance cost. What do you want to copy? Projection is essentially a copy already (unless you reference THIS but in this case you want the mutable object). Otherwise, the method parameters are immutable, so this is not a problem. I guess this was not a very good example;) FindBugs is speaking about the Array, a fool user may initialize the arguments as an array, give it to the method, keeping a reference to the array, to later (concurrently?) change the references to other objects or change positions, so having access to Searche's internal state. My questions is a bit more general, not specifically about this method but about us ignoring this FindBugs warning, be aware of it; I suppose it would be too much paranoia to really change all the dangerous methods, just verifying it is an issue you are aware of; rock solid libraries shouldn't allow it, but it has performance impact. I am fine with trying to enhance but I will not give up of varargs and provided that a session must not be used in a multithreaded way. I am not sure we should pay the price of copying the array in this case. B) there's a STATIC field in SharedReaderProvider: static Field subReadersField it is looking quite suspect, is that really meant to be static? Yes it's on purpose, it's a pointer to a Field object, no need to pay the lookup more than once. Ah sorry I should have looked better, I thought a Lucene Field, not reflection. May I make it final, to enhance readability and to avoid null checks in code? (initializing it with a static helper method) Is the load of the SharedReaderProvider class lazy even with the direct reference to it in ReaderProviderFactory? Otherwise that might trigger the exception for no good purpose. C)Workspace contains many maps to needed objects, all having the same key DirectoryProvider; also locks are mantained in an array; Woudn't it be easier to have a single map, returning a container object with all needed stuff; This object could be passed to all other methods instead of the dir.Provider and avoid many more gets on the maps. also some locking code could go local in this container class, I've some difficulty in tracking the lock sequences. Go ahead (open a JIRA issue), make sure it does no slow things down - it shouldn't from what I've seen) You mean slow down of development time or
