CryptoUtil.java: 1. In CryptoUtil.java, I see a lot of places where the static members are qualified by their full name. For example, content = content.substring(LINE_COUNT); becomes content = content.substring(CryptoUtil.LINE_COUNT); Is this some kind of standard - that local static variables need to be fully qualified? 2. Also in CryptoUtil.java, there are cases where unused variables are removed incorrectly: For example, in public static String reqFormat(String content): int beginIndex = CERTREQ_BEGIN_HEADING.length(); becomes -> CryptoUtil.CERTREQ_BEGIN_HEADING.length(); This happens in multiple places. 3. In public static X509CertInfo createX509CertInfo(), you have the following change: AlgorithmId sigAlgId = new AlgorithmId(); which becomes new AlgorithmId(); and then : info.set(X509CertInfo.ALGORITHM_ID, CertificateAlgorithmId(sigAlgId.get(alg))); becomes: info.set(X509CertInfo.ALGORITHM_ID, new CertificateAlgorithmId(AlgorithmId.get(alg))); Is this really right? The changes in this file as clearly messed up. Please fix and resubmit. HttpEofException.java What is the purpose of the new version UID? HttpRequest.java In this file, you also fully specify some static members. Why? JSSSocketFactory.java 1. You fully qualify a few static members? why? 2. Why does : s.enableSSL2Default(false); become SSLSocket.enableSSL2Default(false); ? 3. Why are we removing comments by findCertByNickname? BasicOCSPResponse.java, CertID.java, GoodInfo.java, KeyHashID.java, NameID.java, OCSPRequest.java, OCSPResponse.java, OCSPResponseStatus, Request.java and many others .. You are qualifying local static members here again. The question is -- is this an automated and hence consistent thing? Is this a standard that needs to be enforced in the code style? UnknownInfo.java 1. Why are we removing comments here? The question is one of consistency -- why are we removing those comments and not the ones in the decode() function? CRSPKIMessage.java 1. Good comments removed 2. Its not clear that the variables here ought to be removed. Utils.java 1. Bad removal of variable in checkHost() -- what is that function doing in any case? Any unit tests? Ade On Wed, 2011-11-02 at 17:15 -0400, Adam Young wrote:

The first of many patches for code _______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel