You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Emmanuel Lecharny <el...@apache.org> on 2009/07/04 01:01:42 UTC
Some comments about the last VYSPER commits
Hi guys,
just a few comments and suggestions :
* http://svn.apache.org/viewvc?rev=790998&view=rev
tests can be defined using annotations, like :
@Test
public void testPublishNoSuchNode() throws Exception {
if Junit 4.4+ is used.
Junit 4.4(+ has the big advantage to allow the use of @beforeClass and @AfterClass annotations, allowing you to run a method before and after *all* the tests. A very powerful feature.
http://svn.apache.org/viewvc/mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/PubsubFeatures.java?rev=791030&view=auto
...
public class PubsubFeatures {
public static final PubsubFeature access_authorize = new PubsubFeature("access-authorize", "The default access model is \"authorize\".", "OPTIONAL", "Nodes Access Models");
... (and all the following)
constants should be uppercased, accordingly to the current coding
convention we are using. These lines should be :
...
public class PubsubFeatures {
public static final PubsubFeature ACCESS_AUTHORIZE = new PubsubFeature("access-authorize", "The default access model is \"authorize\".", "OPTIONAL", "Nodes Access Models");
That helps to keep the code consistant :
...
infoElements.add(new Feature(NamespaceURIs.XEP0060_PUBSUB));
infoElements.add(new Feature(PubsubFeatures.access_open.toString()));
...
Would be better as :
...
infoElements.add(new Feature(NamespaceURIs.XEP0060_PUBSUB));
infoElements.add(new Feature(PubsubFeatures.ACCESS_OPEN.toString()));
...
That's pretty much it, the remaining code is just fine !
Thanks!
( I wish I have time to work on vysper actively ... :/ )
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
Re: Some comments about the last VYSPER commits
Posted by Emmanuel Lecharny <el...@apache.org>.
Michael Jakl wrote:
> Hi!
>
> Thanks for the feedback, good to know that somebody does a sanity
> check from time to time ;)
>
Thanks for the hard work !
Those "sanity checks" is just something we do for every one (but
probably more often for new committers), it's just a way to get a
consistent code base. This is what is the commit mails are good for, btw.
You may discover that different projects at Apache have different code
rules (we at ApacheDS don't use the same rule for '{' and '}' position,
beside other differences). Those 'rules' are per project and have been
discussed a while ago (and this is often the case early in the project's
life).
The cool part is that every PMC can decide to change the rule, assuming
that there is an agreement on that.
In MINA case, almost all the PMC members decided a while back to follow
the standard Java coding style, as expressed on the MINA web site.
It helps killing the religious war about tabs/space, '{' positions and
such stupid little things that makes the life miserable ;)
Thanks Michael, keep the good job going on !
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
Re: Some comments about the last VYSPER commits
Posted by Michael Jakl <ja...@gmail.com>.
Hi!
Thanks for the feedback, good to know that somebody does a sanity
check from time to time ;)
On Sat, Jul 4, 2009 at 01:01, Emmanuel Lecharny<el...@apache.org> wrote:
> tests can be defined using annotations, like :
>
> @Test
> public void testPublishNoSuchNode() throws Exception {
>
> if Junit 4.4+ is used.
> Junit 4.4(+ has the big advantage to allow the use of @beforeClass and
> @AfterClass annotations, allowing you to run a method before and after *all*
> the tests. A very powerful feature.
Currently we're only using JUnit 3, which doesn't support any of these
annotations.
Concerning before-/afterClass methods: I think that a test should be
as independent as possible. As long as we're not experiencing serious
setUp and tearDown times I'd rather have each test setup its own
environment.
Our whole test-suite runs in 1.5 seconds on my computer. Most of the
time I'm executing only the tests for pubsub or even the test for the
feature I'm implementing, which takes even less time.
Of course these annotations are nice and methods could be given more
intuitive names, ATM this would cause more changes than just switching
the junit library.
> http://svn.apache.org/viewvc/mina/sandbox/vysper/trunk/src/main/java/org/apache/vysper/xmpp/modules/extension/xep0060_pubsub/PubsubFeatures.java?rev=791030&view=auto
>
> ...
> public class PubsubFeatures {
> public static final PubsubFeature access_authorize = new
> PubsubFeature("access-authorize", "The default access model is
> \"authorize\".", "OPTIONAL", "Nodes Access Models");
> ... (and all the following)
>
> constants should be uppercased, accordingly to the current coding convention
> we are using. These lines should be :
>
> ...
> public class PubsubFeatures {
> public static final PubsubFeature ACCESS_AUTHORIZE = new
> PubsubFeature("access-authorize", "The default access model is
> \"authorize\".", "OPTIONAL", "Nodes Access Models");
>
> That helps to keep the code consistant :
>
> ...
> infoElements.add(new Feature(NamespaceURIs.XEP0060_PUBSUB));
> infoElements.add(new Feature(PubsubFeatures.access_open.toString()));
> ...
>
> Would be better as :
>
> ...
> infoElements.add(new Feature(NamespaceURIs.XEP0060_PUBSUB));
> infoElements.add(new Feature(PubsubFeatures.ACCESS_OPEN.toString()));
> ...
Thanks, done. The file was generated semi-automatically and I was too
lazy to go over the names and change them to uppercase yesterday.
> That's pretty much it, the remaining code is just fine !
Thanks!
Michael