Re: [EGIT PATCH] Fixed trivial warnings. Mainly parametrized raw types, added serialVersionUID, removed unnecessery throws.

[Vasyl' Vavrychuk <vvavrychuk@gmail.com>]

Robin Rosenberg wrote:
> söndag 04 januari 2009 23:20:19 skrev Vasyl' Vavrychuk:
>> Also fixed:
>> 1. "The 'Eclipse-LazyStart' header is deprecated, use 'Bundle-ActivationPolicy'" warning.
>> 2. Possible NullPointerException warning.
>> 3. Unnecessery function parameter warning.
> 
> Thanks for your interest in the projects. 
> Most changes from a 30 seconds review look reasonable.
> However we don't apply changes this way. From your comments it seems we'd require about
> five patches since the changes are of very different types.

I thought that commit is trivial. But maybe series of patches will be better because of an ability to revert what we want.

> 
> -- robin
> 
>> diff --git a/org.spearce.egit.core.test/META-INF/MANIFEST.MF b/org.spearce.egit.core.test/META-INF/MANIFEST.MF
>> index ee5f277..e8bcc79 100644
>> --- a/org.spearce.egit.core.test/META-INF/MANIFEST.MF
>> +++ b/org.spearce.egit.core.test/META-INF/MANIFEST.MF
>> @@ -11,7 +11,7 @@ Require-Bundle: org.eclipse.core.runtime,
>>   org.spearce.egit.ui,
>>   org.spearce.jgit,
>>   org.eclipse.core.filesystem
>> -Eclipse-LazyStart: true
>> +Bundle-ActivationPolicy: lazy
> 
> Any pointers on this? (so I can learn)
http://help.eclipse.org/ganymede/index.jsp?topic=/org.eclipse.platform.doc.isv/reference/misc/bundle_manifest.html
The Eclipse-AutoStart and Eclipse-LazyStart headers have been deprecated in Eclipse 3.4

> 
>> diff --git a/org.spearce.egit.core/src/org/spearce/egit/core/internal/storage/GitFileHistory.java b/org.spearce.egit.core/src/org/spearce/egit/core/internal/storage/GitFileHistory.java
>> index c01c1c3..61c32ce 100644
>> --- a/org.spearce.egit.core/src/org/spearce/egit/core/internal/storage/GitFileHistory.java
>> +++ b/org.spearce.egit.core/src/org/spearce/egit/core/internal/storage/GitFileHistory.java
>> @@ -50,11 +50,11 @@
>>  	GitFileHistory(final IResource rsrc, final int flags,
>>  			final IProgressMonitor monitor) {
>>  		resource = rsrc;
>> -		walk = buildWalk(flags);
>> +		walk = buildWalk(/*flags*/);
>>  		revisions = buildRevisions(monitor, flags);
>>  	}
>> -	private KidWalk buildWalk(final int flags) {
>> +	private KidWalk buildWalk(/*final int flags*/) {
> Can't we just drop the parameter completely and wipe every trace of it?
OK

>>  		final RepositoryMapping rm = RepositoryMapping.getMapping(resource);
>>  		if (rm == null) {
>>  			Activator.logError("Git not attached to project "
>> diff --git a/org.spearce.egit.core/src/org/spearce/egit/core/project/GitProjectData.java b/org.spearce.egit.core/src/org/spearce/egit/core/project/GitProjectData.java
>> index 04130db..db5f20b 100644
>> --- a/org.spearce.egit.core/src/org/spearce/egit/core/project/GitProjectData.java
>> +++ b/org.spearce.egit.core/src/org/spearce/egit/core/project/GitProjectData.java
>> @@ -48,9 +48,9 @@
>>   * a Git repository.
>>   */
>>  public class GitProjectData {
>> -	private static final Map projectDataCache = new HashMap();
>> +	private static final Map<IProject, GitProjectData> projectDataCache = new HashMap<IProject, GitProjectData>();
>>  
>> -	private static final Map repositoryCache = new HashMap();
>> +	private static final Map<File, WeakReference> repositoryCache = new HashMap<File, WeakReference>();
> Been thinking about doing this myself but always found something more interesting to do. Good.

Hope that we will have a generic version of SWT/JFace sometime. Because now I do not know how to handle an inter operation between generic collections and SWT, use SuppressWarning there maybe?

>> diff --git a/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java b/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
>> index dcc53cd..4700510 100644
>> --- a/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
>> +++ b/org.spearce.jgit/src/org/spearce/jgit/treewalk/CanonicalTreeParser.java
>> @@ -175,8 +175,9 @@ public void back(int delta) {
>>  			// space so this prunes our search more quickly.
>>  			//
>>  			ptr -= Constants.OBJECT_ID_LENGTH;
>> -			while (raw[--ptr] != ' ')
>> -				/* nothing */;
>> +			while (raw[--ptr] != ' ') {
>> +				/* nothing */
>> +			}
> Not sure if this buys us anything. I wouldn't react if original code was either way, but
> changing it seems unnecessary.

With an old version I get "Empty control-flow statement" warning, I've checked org.eclipse.jdt.core.prefs file, there is:
org.eclipse.jdt.core.compiler.problem.emptyStatement=warning