* [EGIT PATCH] Comment private modifier to improve performace.
@ 2008-02-02 2:23 Roger C. Soares
2008-02-03 1:01 ` Robin Rosenberg
0 siblings, 1 reply; 7+ messages in thread
From: Roger C. Soares @ 2008-02-02 2:23 UTC (permalink / raw)
To: git; +Cc: robin.rosenberg, Roger C. Soares
Changed private modifiers to default to improve perfomance and remove
warnings of the type:
Write access to enclosing field GitHistoryPage.hintShowDiffNow is
emulated by a synthetic accessor method. Increasing its visibility will
improve your performance
Signed-off-by: Roger C. Soares <rogersoares@intelinet.com.br>
---
.../egit/core/internal/mapping/GitFileHistory.java | 2 +-
.../src/org/spearce/egit/ui/GitHistoryPage.java | 2 +-
.../internal/actions/AbstractOperationAction.java | 2 +-
.../internal/decorators/GitResourceDecorator.java | 2 +-
.../ui/internal/dialogs/BranchSelectionDialog.java | 2 +-
.../egit/ui/internal/dialogs/CommitDialog.java | 2 +-
.../src/org/spearce/jgit/lib/GitIndex.java | 14 +++++++-------
.../src/org/spearce/jgit/lib/Walker.java | 12 ++++++------
.../src/org/spearce/jgit/lib/WindowedFile.java | 6 +++---
.../src/org/spearce/jgit/lib/WorkDirCheckout.java | 2 +-
10 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/org.spearce.egit.core/src/org/spearce/egit/core/internal/mapping/GitFileHistory.java b/org.spearce.egit.core/src/org/spearce/egit/core/internal/mapping/GitFileHistory.java
index 889ca0c..9e683a5 100644
--- a/org.spearce.egit.core/src/org/spearce/egit/core/internal/mapping/GitFileHistory.java
+++ b/org.spearce.egit.core/src/org/spearce/egit/core/internal/mapping/GitFileHistory.java
@@ -107,7 +107,7 @@ public class GitFileHistory extends FileHistory implements IAdaptable {
IResource resource;
private final IProgressMonitor monitor;
- private Map<ObjectId, IFileRevision> revisions = new HashMap<ObjectId, IFileRevision>();
+ /* private */Map<ObjectId, IFileRevision> revisions = new HashMap<ObjectId, IFileRevision>();
EclipseWalker(Repository repository, Commit[] starts, String[] relativeResourceName,boolean leafIsBlob,IResource resource,boolean followMainOnly, Boolean merges, ObjectId lastActiveDiffId, boolean returnAll, IProgressMonitor monitor) {
super(repository, starts, relativeResourceName, leafIsBlob, followMainOnly, merges, lastActiveDiffId, returnAll);
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/GitHistoryPage.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/GitHistoryPage.java
index 812747d..1f154a5 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/GitHistoryPage.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/GitHistoryPage.java
@@ -137,7 +137,7 @@ public class GitHistoryPage extends HistoryPage implements IAdaptable,
/* private */List<IFileRevision> fileRevisions;
- private boolean hintShowDiffNow;
+ /* private */boolean hintShowDiffNow;
private boolean showAllProjectVersions;
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/AbstractOperationAction.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/AbstractOperationAction.java
index c3f347e..d8e1961 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/AbstractOperationAction.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/AbstractOperationAction.java
@@ -42,7 +42,7 @@ import org.spearce.egit.ui.UIText;
public abstract class AbstractOperationAction implements IObjectActionDelegate {
private IWorkbenchPart wp;
- private IWorkspaceRunnable op;
+ /* private */IWorkspaceRunnable op;
public void selectionChanged(final IAction act, final ISelection sel) {
final List selection;
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
index c13c38a..cc387bc 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java
@@ -67,7 +67,7 @@ import org.spearce.jgit.lib.Repository.RepositoryState;
public class GitResourceDecorator extends LabelProvider implements
ILightweightLabelDecorator {
- private static final RCL myrcl = new RCL();
+ /* private */static final RCL myrcl = new RCL();
static class RCL implements RepositoryChangeListener, Runnable {
private boolean requested;
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
index 5e30027..2d140a4 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/BranchSelectionDialog.java
@@ -215,7 +215,7 @@ public class BranchSelectionDialog extends Dialog {
return refName;
}
- private ResetType resetType = ResetType.MIXED;
+ /* private */ResetType resetType = ResetType.MIXED;
/**
* @return Type of Reset
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
index 3778b94..847666f 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
@@ -327,7 +327,7 @@ public class CommitDialog extends Dialog {
private boolean amendAllowed = true;
private ArrayList<IFile> selectedItems = new ArrayList<IFile>();
- private String previousCommitMessage = "";
+ /* private */String previousCommitMessage = "";
/**
* Pre-select suggested set of resources to commit
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/GitIndex.java b/org.spearce.jgit/src/org/spearce/jgit/lib/GitIndex.java
index f49370c..bdcb71a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/GitIndex.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/GitIndex.java
@@ -64,7 +64,7 @@ public class GitIndex {
private long lastCacheTime;
- private final Repository db;
+ /* private */final Repository db;
private Map entries = new TreeMap(new Comparator() {
public int compare(Object arg0, Object arg1) {
@@ -311,7 +311,7 @@ public class GitIndex {
}
Boolean filemode;
- private boolean config_filemode() {
+ /* private */boolean config_filemode() {
// temporary til we can actually set parameters. We need to be able
// to change this for testing.
if (filemode != null)
@@ -322,15 +322,15 @@ public class GitIndex {
/** An index entry */
public class Entry {
- private long ctime;
+ /* private */long ctime;
- private long mtime;
+ /* private */long mtime;
private int dev;
private int ino;
- private int mode;
+ /* private */int mode;
private int uid;
@@ -338,11 +338,11 @@ public class GitIndex {
private int size;
- private ObjectId sha1;
+ /* private */ObjectId sha1;
private short flags;
- private byte[] name;
+ /* private */byte[] name;
Entry(byte[] key, File f, int stage)
throws IOException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Walker.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Walker.java
index 1f6c531..eb824e0 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Walker.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Walker.java
@@ -17,15 +17,15 @@ import java.util.Map;
* data according to some criteria.
*/
public abstract class Walker {
- private String[] relativeResourceName;
- private boolean leafIsBlob;
- private boolean followMainOnly;
+ /* private */String[] relativeResourceName;
+ /* private */boolean leafIsBlob;
+ /* private */boolean followMainOnly;
protected Repository repository;
private ObjectId activeDiffLeafId;
protected final Commit[] starts;
- private final Boolean merges;
- private Map donewith = new ObjectIdMap();
- private Collection<Todo> todo = new ArrayList<Todo>(20000);
+ /* private */final Boolean merges;
+ /* private */Map donewith = new ObjectIdMap();
+ /* private */Collection<Todo> todo = new ArrayList<Todo>(20000);
protected abstract boolean isCancelled();
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
index 39f1477..13fecad 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
@@ -66,15 +66,15 @@ public class WindowedFile {
private final WindowCache cache;
- private final int sz;
+ /* private */final int sz;
- private final int szb;
+ /* private */final int szb;
private final int szm;
private final Provider wp;
- private final long length;
+ /* private */final long length;
/**
* Open a file for reading through window caching.
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WorkDirCheckout.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WorkDirCheckout.java
index 926dac3..6159469 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WorkDirCheckout.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WorkDirCheckout.java
@@ -362,7 +362,7 @@ public class WorkDirCheckout {
return hasParentBlob(t, parent);
}
- private void checkConflictsWithFile(File file) {
+ /* private */void checkConflictsWithFile(File file) {
if (file.isDirectory()) {
ArrayList<String> childFiles = listFiles(file);
conflicts.addAll(childFiles);
--
1.5.3.7
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [EGIT PATCH] Comment private modifier to improve performace.
2008-02-02 2:23 [EGIT PATCH] Comment private modifier to improve performace Roger C. Soares
@ 2008-02-03 1:01 ` Robin Rosenberg
2008-02-03 2:26 ` Robin Rosenberg
2008-02-03 19:46 ` Roger C. Soares
0 siblings, 2 replies; 7+ messages in thread
From: Robin Rosenberg @ 2008-02-03 1:01 UTC (permalink / raw)
To: Roger C. Soares; +Cc: git
lördagen den 2 februari 2008 skrev Roger C. Soares:
> Changed private modifiers to default to improve perfomance and remove
> warnings of the type:
> Write access to enclosing field GitHistoryPage.hintShowDiffNow is
> emulated by a synthetic accessor method. Increasing its visibility will
> improve your performance
I'm not fully convinced this is the right way after all. Good
performance is obviously good, but so is good encapsulation.
I've sometimes tried changing things like this even in pieces
of code that I really thought it should matter, but not been able
to measure any real improvemen even with performance measurment tools.
Obviously seeing that warning is annoying so maybe we should just set it to
ignore or exclude it from the project settings (if that is possible). The
only project where I think it might make a difference is the jgit part because
that is where we optimize and that is where I experimented with visibility
changes. In the Eclipse part we need to encapsulate more, partly because
Eclipse is less understood by the current authors than Java in general.
Encapsulation means encapsulating bad coding and bad design that comes
from lack of understanding of the framework we are working within.
-- robin
From f26deb2c14e1df7513b3954594ea09b7746fcb69 Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Sun, 3 Feb 2008 01:46:33 +0100
Subject: [PATCH] Ignore warning for "Access to enclosing method/field X emulated by a synthetic accessor method"
There is no measurable performance degradation from this. Increasing visibility
of methods and fields in order to get rid of this warning reduces encapsulation.
Instead of choosing the Ignore setting we remove the setting from the project,
which may be somwhat hackish but has the benefit that the setting can be changed
in the workspace for those that wants to investigate the issue.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
5 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
index deec031..7a0fbe4 100644
--- a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
@@ -55,7 +55,6 @@ org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.specialParameterHidingField=disabled
org.eclipse.jdt.core.compiler.problem.staticAccessReceiver=error
org.eclipse.jdt.core.compiler.problem.suppressWarnings=enabled
-org.eclipse.jdt.core.compiler.problem.syntheticAccessEmulation=warning
org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
diff --git a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
index fa7a0b7..bcde160 100644
--- a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
@@ -55,7 +55,6 @@ org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.specialParameterHidingField=disabled
org.eclipse.jdt.core.compiler.problem.staticAccessReceiver=error
org.eclipse.jdt.core.compiler.problem.suppressWarnings=enabled
-org.eclipse.jdt.core.compiler.problem.syntheticAccessEmulation=warning
org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
diff --git a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
index 0ef7591..0a89f52 100644
--- a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
@@ -55,7 +55,6 @@ org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.specialParameterHidingField=disabled
org.eclipse.jdt.core.compiler.problem.staticAccessReceiver=error
org.eclipse.jdt.core.compiler.problem.suppressWarnings=enabled
-org.eclipse.jdt.core.compiler.problem.syntheticAccessEmulation=warning
org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
diff --git a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
index 65d5c31..c203c20 100644
--- a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
@@ -55,7 +55,6 @@ org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.specialParameterHidingField=disabled
org.eclipse.jdt.core.compiler.problem.staticAccessReceiver=error
org.eclipse.jdt.core.compiler.problem.suppressWarnings=enabled
-org.eclipse.jdt.core.compiler.problem.syntheticAccessEmulation=warning
org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
diff --git a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
index 4c38185..b0c694c 100644
--- a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
@@ -55,7 +55,6 @@ org.eclipse.jdt.core.compiler.problem.redundantNullCheck=warning
org.eclipse.jdt.core.compiler.problem.specialParameterHidingField=disabled
org.eclipse.jdt.core.compiler.problem.staticAccessReceiver=error
org.eclipse.jdt.core.compiler.problem.suppressWarnings=enabled
-org.eclipse.jdt.core.compiler.problem.syntheticAccessEmulation=warning
org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
--
1.5.4.rc4.25.g81cc
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [EGIT PATCH] Comment private modifier to improve performace.
2008-02-03 1:01 ` Robin Rosenberg
@ 2008-02-03 2:26 ` Robin Rosenberg
2008-02-03 20:03 ` Roger C. Soares
2008-02-03 19:46 ` Roger C. Soares
1 sibling, 1 reply; 7+ messages in thread
From: Robin Rosenberg @ 2008-02-03 2:26 UTC (permalink / raw)
To: Roger C. Soares; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 64 bytes --]
We could drop these settings from the projects too.
-- robin
[-- Attachment #2: 0002-Make-it-possible-to-ignore-warnings-about-discourage.patch --]
[-- Type: text/x-diff, Size: 5084 bytes --]
From a1aed72a122a05d01ed57db5d51b45debeb247b7 Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Sun, 3 Feb 2008 03:13:44 +0100
Subject: [PATCH] Make it possible to ignore warnings about discouraged access
By removing the setting from the project the workspace settings
will be applied (default: Warning)
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
5 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
index 7a0fbe4..1c978ca 100644
--- a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
@@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
org.eclipse.jdt.core.compiler.problem.deprecation=warning
org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
-org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
diff --git a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
index bcde160..2afc050 100644
--- a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
@@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
org.eclipse.jdt.core.compiler.problem.deprecation=warning
org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
-org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
diff --git a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
index 0a89f52..0da92c7 100644
--- a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
@@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
org.eclipse.jdt.core.compiler.problem.deprecation=warning
org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
-org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
diff --git a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
index c203c20..9adf651 100644
--- a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
@@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
org.eclipse.jdt.core.compiler.problem.deprecation=warning
org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
-org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
diff --git a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
index b0c694c..faecf76 100644
--- a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
@@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
org.eclipse.jdt.core.compiler.problem.deprecation=warning
org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
-org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
--
1.5.4.rc4.25.g81cc
[-- Attachment #3: 0003-Drop-warnings-about-unnecessary-else.patch --]
[-- Type: text/x-diff, Size: 5304 bytes --]
From e9f312599eb5941a7bf1acd70c6f0ac9fb1ea889 Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Sun, 3 Feb 2008 03:19:09 +0100
Subject: [PATCH] Drop warnings about unnecessary else
Use workspace setting, default ignore.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 1 -
.../.settings/org.eclipse.jdt.core.prefs | 3 +--
5 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
index 1c978ca..7279d81 100644
--- a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
@@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
-org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
diff --git a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
index 2afc050..2d94530 100644
--- a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
@@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
-org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
diff --git a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
index 0da92c7..ff09222 100644
--- a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
@@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
-org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
diff --git a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
index 9adf651..b93093f 100644
--- a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
@@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
-org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
diff --git a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
index faecf76..4a5d15f 100644
--- a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
+++ b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
@@ -1,4 +1,4 @@
-#Tue Dec 18 01:35:52 CET 2007
+#Sun Feb 03 03:16:45 CET 2008
eclipse.preferences.version=1
org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled
org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.5
@@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
-org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
--
1.5.4.rc4.25.g81cc
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [EGIT PATCH] Comment private modifier to improve performace.
2008-02-03 1:01 ` Robin Rosenberg
2008-02-03 2:26 ` Robin Rosenberg
@ 2008-02-03 19:46 ` Roger C. Soares
2008-02-03 22:25 ` Robin Rosenberg
1 sibling, 1 reply; 7+ messages in thread
From: Roger C. Soares @ 2008-02-03 19:46 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
Robin Rosenberg escreveu:
>
> I'm not fully convinced this is the right way after all. Good
> performance is obviously good, but so is good encapsulation.
> I've sometimes tried changing things like this even in pieces
> of code that I really thought it should matter, but not been able
> to measure any real improvemen even with performance measurment tools.
>
> Obviously seeing that warning is annoying so maybe we should just set it to
> ignore or exclude it from the project settings (if that is possible). The
> only project where I think it might make a difference is the jgit part because
> that is where we optimize and that is where I experimented with visibility
> changes. In the Eclipse part we need to encapsulate more, partly because
> Eclipse is less understood by the current authors than Java in general.
> Encapsulation means encapsulating bad coding and bad design that comes
> from lack of understanding of the framework we are working within.
>
>
Ok, so some more points for your consideration:
. I saw this /* private */ notation on eclipse code and found it
interesting.
. I won't bother measuaring any single case to make sure it is not
impacting performance under some circunstance, so resolving those
warnings puts me in the safe area. On the other hand, I think it is a
lot easier to tell if a patch is breaking encapsulation in a bad way
just by reviewing it, which is something that is already done.
Especially if it has the private modifier commented out. Someone can
even do a script to uncomment them and verify that it still builds
without errors.
. The ui part isn't supposed to be reused by other projects, so I think
encapsulation there is less important than for jgit. But even so, the
default modifier (or package-private) is good enough for encapsulation.
Other projects shouldn't write code in the the same packages from jgit,
if they do so they know that they are using internal things and they can
run into problems in the future.
That said, I'm ok with your patch too. I think the important is to
choose one so we stop mixing things, for consistency's sake.
[]s,
Roger.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [EGIT PATCH] Comment private modifier to improve performace.
2008-02-03 2:26 ` Robin Rosenberg
@ 2008-02-03 20:03 ` Roger C. Soares
2008-02-03 22:14 ` Robin Rosenberg
0 siblings, 1 reply; 7+ messages in thread
From: Roger C. Soares @ 2008-02-03 20:03 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
With the else warnings patch I'm ok.
About the discouraged access, I read those warnings as: we are using
methods that are not part of the eclipse public API and they can change
in the future. Not depending on internal eclipse API will make egit less
likely to break with a future eclipse version, which sounds like a good
thing to me. So, I would keep those.
[]s,
Roger.
--
Robin Rosenberg escreveu:
> We could drop these settings from the projects too.
>
> -- robin
>
> ------------------------------------------------------------------------
>
> From a1aed72a122a05d01ed57db5d51b45debeb247b7 Mon Sep 17 00:00:00 2001
> From: Robin Rosenberg <robin.rosenberg@dewire.com>
> Date: Sun, 3 Feb 2008 03:13:44 +0100
> Subject: [PATCH] Make it possible to ignore warnings about discouraged access
>
> By removing the setting from the project the workspace settings
> will be applied (default: Warning)
>
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
> .../.settings/org.eclipse.jdt.core.prefs | 1 -
> .../.settings/org.eclipse.jdt.core.prefs | 1 -
> .../.settings/org.eclipse.jdt.core.prefs | 1 -
> .../.settings/org.eclipse.jdt.core.prefs | 1 -
> .../.settings/org.eclipse.jdt.core.prefs | 1 -
> 5 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
> index 7a0fbe4..1c978ca 100644
> --- a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
> @@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
> org.eclipse.jdt.core.compiler.problem.deprecation=warning
> org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
> org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
> -org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
> org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
> org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
> org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
> diff --git a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
> index bcde160..2afc050 100644
> --- a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
> @@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
> org.eclipse.jdt.core.compiler.problem.deprecation=warning
> org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
> org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
> -org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
> org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
> org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
> org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
> diff --git a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
> index 0a89f52..0da92c7 100644
> --- a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
> @@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
> org.eclipse.jdt.core.compiler.problem.deprecation=warning
> org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
> org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
> -org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
> org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
> org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
> org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
> diff --git a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
> index c203c20..9adf651 100644
> --- a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
> @@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
> org.eclipse.jdt.core.compiler.problem.deprecation=warning
> org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
> org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
> -org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
> org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
> org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
> org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
> diff --git a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
> index b0c694c..faecf76 100644
> --- a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
> @@ -14,7 +14,6 @@ org.eclipse.jdt.core.compiler.problem.autoboxing=warning
> org.eclipse.jdt.core.compiler.problem.deprecation=warning
> org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode=disabled
> org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod=disabled
> -org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
> org.eclipse.jdt.core.compiler.problem.emptyStatement=warning
> org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
> org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
>
> ------------------------------------------------------------------------
>
> From e9f312599eb5941a7bf1acd70c6f0ac9fb1ea889 Mon Sep 17 00:00:00 2001
> From: Robin Rosenberg <robin.rosenberg@dewire.com>
> Date: Sun, 3 Feb 2008 03:19:09 +0100
> Subject: [PATCH] Drop warnings about unnecessary else
>
> Use workspace setting, default ignore.
>
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
> .../.settings/org.eclipse.jdt.core.prefs | 1 -
> .../.settings/org.eclipse.jdt.core.prefs | 1 -
> .../.settings/org.eclipse.jdt.core.prefs | 1 -
> .../.settings/org.eclipse.jdt.core.prefs | 1 -
> .../.settings/org.eclipse.jdt.core.prefs | 3 +--
> 5 files changed, 1 insertions(+), 6 deletions(-)
>
> diff --git a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
> index 1c978ca..7279d81 100644
> --- a/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.egit.core.test/.settings/org.eclipse.jdt.core.prefs
> @@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
> org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
> org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
> org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
> -org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
> org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
> org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
> org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
> diff --git a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
> index 2afc050..2d94530 100644
> --- a/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.egit.core/.settings/org.eclipse.jdt.core.prefs
> @@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
> org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
> org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
> org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
> -org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
> org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
> org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
> org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
> diff --git a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
> index 0da92c7..ff09222 100644
> --- a/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.egit.ui/.settings/org.eclipse.jdt.core.prefs
> @@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
> org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
> org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
> org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
> -org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
> org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
> org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
> org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
> diff --git a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
> index 9adf651..b93093f 100644
> --- a/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.jgit.test/.settings/org.eclipse.jdt.core.prefs
> @@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
> org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
> org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
> org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
> -org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
> org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
> org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
> org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
> diff --git a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
> index faecf76..4a5d15f 100644
> --- a/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
> +++ b/org.spearce.jgit/.settings/org.eclipse.jdt.core.prefs
> @@ -1,4 +1,4 @@
> -#Tue Dec 18 01:35:52 CET 2007
> +#Sun Feb 03 03:16:45 CET 2008
> eclipse.preferences.version=1
> org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled
> org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.5
> @@ -58,7 +58,6 @@ org.eclipse.jdt.core.compiler.problem.typeParameterHiding=warning
> org.eclipse.jdt.core.compiler.problem.uncheckedTypeOperation=warning
> org.eclipse.jdt.core.compiler.problem.undocumentedEmptyBlock=warning
> org.eclipse.jdt.core.compiler.problem.unhandledWarningToken=warning
> -org.eclipse.jdt.core.compiler.problem.unnecessaryElse=warning
> org.eclipse.jdt.core.compiler.problem.unnecessaryTypeCheck=error
> org.eclipse.jdt.core.compiler.problem.unqualifiedFieldAccess=ignore
> org.eclipse.jdt.core.compiler.problem.unusedDeclaredThrownException=error
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [EGIT PATCH] Comment private modifier to improve performace.
2008-02-03 20:03 ` Roger C. Soares
@ 2008-02-03 22:14 ` Robin Rosenberg
0 siblings, 0 replies; 7+ messages in thread
From: Robin Rosenberg @ 2008-02-03 22:14 UTC (permalink / raw)
To: Roger C. Soares; +Cc: git
söndagen den 3 februari 2008 skrev Roger C. Soares:
> With the else warnings patch I'm ok.
>
> About the discouraged access, I read those warnings as: we are using
> methods that are not part of the eclipse public API and they can change
> in the future. Not depending on internal eclipse API will make egit less
> likely to break with a future eclipse version, which sounds like a good
> thing to me. So, I would keep those.
The thing with that one is that by default the warning is on, so removing
the setting doesn't remvoe the warning unless one disables it at the workspace
level. I'll hold it back for the moment anyway and push the "else" and "synthetic"
patches. Thanks for your opinions on the subject matter.
-- robin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [EGIT PATCH] Comment private modifier to improve performace.
2008-02-03 19:46 ` Roger C. Soares
@ 2008-02-03 22:25 ` Robin Rosenberg
0 siblings, 0 replies; 7+ messages in thread
From: Robin Rosenberg @ 2008-02-03 22:25 UTC (permalink / raw)
To: Roger C. Soares; +Cc: git
söndagen den 3 februari 2008 skrev Roger C. Soares:
> Ok, so some more points for your consideration:
>
> . I saw this /* private */ notation on eclipse code and found it
> interesting.
It is.
> . I won't bother measuaring any single case to make sure it is not
> impacting performance under some circunstance, so resolving those
> warnings puts me in the safe area. On the other hand, I think it is a
> lot easier to tell if a patch is breaking encapsulation in a bad way
> just by reviewing it, which is something that is already done.
We do not have a large number of people reviewing our code at this stage,
so I would not trust that at the moment.
> Especially if it has the private modifier commented out. Someone can
> even do a script to uncomment them and verify that it still builds
> without errors.
There is more to reviewing than just looking at the diffs.
> . The ui part isn't supposed to be reused by other projects, so I think
> encapsulation there is less important than for jgit. But even so, the
> default modifier (or package-private) is good enough for encapsulation.
> Other projects shouldn't write code in the the same packages from jgit,
> if they do so they know that they are using internal things and they can
> run into problems in the future.
I'm thinking more about internal encapsulation between the classes within the
project. Many of the classes in the ui (and jgit) packages have little in
common other than that they contribute to the same overall goal (an Eclipse
plugin). I guess that is what happens when things get build a small patch at
a time. Obviously we could have more packages, but that might make things
worse by forcing us to have more public methods and having a lot of one and
two class packages feels wrong too (though it actually may be right). I don't
think I'll look at that right now though.
-- robin
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-02-03 22:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-02 2:23 [EGIT PATCH] Comment private modifier to improve performace Roger C. Soares
2008-02-03 1:01 ` Robin Rosenberg
2008-02-03 2:26 ` Robin Rosenberg
2008-02-03 20:03 ` Roger C. Soares
2008-02-03 22:14 ` Robin Rosenberg
2008-02-03 19:46 ` Roger C. Soares
2008-02-03 22:25 ` Robin Rosenberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).