You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oltu.apache.org by Simone Tripodi <si...@apache.org> on 2014/04/11 13:51:15 UTC

Re: svn commit: r1583609 - in /oltu/trunk/jose/jws: ./ src/main/java/org/apache/oltu/jose/jws/ src/main/java/org/apache/oltu/jose/jws/signature/impl/

Hi Tonino,

just few considerations:

> +public class JwsConstants {
> +
> +    public static final String RS256 = "RS256";
> +
> +    public static final String RS384 = "RS384";
> +
> +    public static final String RS512 = "RS512";
> +}

I'd reduce this class constructor as 'private'

>
> Added: oltu/trunk/jose/jws/src/main/java/org/apache/oltu/jose/jws/signature/impl/PrivateKey.java

I wouldn't add that classes to a generic 'impl' package, they refer to
specific 'java.security' implementation, so I would suggest to:

 * having them implemented in a separated module/bundle

 * package name be renamed

WDYT?
Best,
-Simo

Re: svn commit: r1583609 - in /oltu/trunk/jose/jws: ./ src/main/java/org/apache/oltu/jose/jws/ src/main/java/org/apache/oltu/jose/jws/signature/impl/

Posted by Łukasz Dywicki <lu...@code-house.org>.
If thiss class contains just constants you can make it as interface and skip repeated "public static final” and also avoid checkstyle crying about constructor. :)

Cheers,
Łukasz Dywicki
--
luke@code-house.org
Twitter: ldywicki
Blog: http://dywicki.pl
Code-House - http://code-house.org

Wiadomość napisana przez Antonio Sanso <as...@adobe.com> w dniu 11 kwi 2014, o godz. 14:45:

> hi Simo +1
> 
> Maybe we can leave in the same bundle though. Just not in the impl package…
> 
> regards
> 
> antonio
> 
> On Apr 11, 2014, at 1:51 PM, Simone Tripodi <si...@apache.org> wrote:
> 
>> Hi Tonino,
>> 
>> just few considerations:
>> 
>>> +public class JwsConstants {
>>> +
>>> +    public static final String RS256 = "RS256";
>>> +
>>> +    public static final String RS384 = "RS384";
>>> +
>>> +    public static final String RS512 = "RS512";
>>> +}
>> 
>> I'd reduce this class constructor as 'private'
>> 
>>> 
>>> Added: oltu/trunk/jose/jws/src/main/java/org/apache/oltu/jose/jws/signature/impl/PrivateKey.java
>> 
>> I wouldn't add that classes to a generic 'impl' package, they refer to
>> specific 'java.security' implementation, so I would suggest to:
>> 
>> * having them implemented in a separated module/bundle
>> 
>> * package name be renamed
>> 
>> WDYT?
>> Best,
>> -Simo
>

Re: svn commit: r1583609 - in /oltu/trunk/jose/jws: ./ src/main/java/org/apache/oltu/jose/jws/ src/main/java/org/apache/oltu/jose/jws/signature/impl/

Posted by Antonio Sanso <as...@adobe.com>.
hi Simo +1

Maybe we can leave in the same bundle though. Just not in the impl package…

regards

antonio

On Apr 11, 2014, at 1:51 PM, Simone Tripodi <si...@apache.org> wrote:

> Hi Tonino,
> 
> just few considerations:
> 
>> +public class JwsConstants {
>> +
>> +    public static final String RS256 = "RS256";
>> +
>> +    public static final String RS384 = "RS384";
>> +
>> +    public static final String RS512 = "RS512";
>> +}
> 
> I'd reduce this class constructor as 'private'
> 
>> 
>> Added: oltu/trunk/jose/jws/src/main/java/org/apache/oltu/jose/jws/signature/impl/PrivateKey.java
> 
> I wouldn't add that classes to a generic 'impl' package, they refer to
> specific 'java.security' implementation, so I would suggest to:
> 
> * having them implemented in a separated module/bundle
> 
> * package name be renamed
> 
> WDYT?
> Best,
> -Simo