From: Robin Rosenberg <robin.rosenberg@dewire.com>
To: "Roger C. Soares" <rogersoares@intelinet.com.br>
Cc: git@vger.kernel.org
Subject: Re: [EGIT PATCH] Comment private modifier to improve performace.
Date: Sun, 3 Feb 2008 23:25:47 +0100 [thread overview]
Message-ID: <200802032325.47844.robin.rosenberg@dewire.com> (raw)
In-Reply-To: <47A61A30.3000904@intelinet.com.br>
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
prev parent reply other threads:[~2008-02-03 22:26 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200802032325.47844.robin.rosenberg@dewire.com \
--to=robin.rosenberg@dewire.com \
--cc=git@vger.kernel.org \
--cc=rogersoares@intelinet.com.br \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).