https://issues.apache.org/bugzilla/show_bug.cgi?id=57021

--- Comment #16 from VIN <reachme.va...@gmail.com> ---
(In reply to Mark Thomas from comment #10)
> Some further feedback
> 
> 1. Indents should be with 4 spaces not tabs.
     Hi Mark,
     I changed the code using Eclipse editor which by default adds tab as
indent. Next time on wards i will open the changed code in notepad and check
the indents.

> 
> 2. In LibraryNotFoundError libraryName is better than nameOfLibrary
     Sure, Thanks.
> 
> 3. Improvement B requested name*s* of the libraries, not just one.
     As per the enhancement mentioned in the request, Tomcat code should not
try to load the second library when the first library failed to load. So I
thought there is no point in maintaining array of library names.

Please correct me if my understanding is wrong.

> 
> 4. The patch always throws an Exception on the first name. There is never a
> chance to check the second name.

     As per the enhancement mentioned in request, code should not try load the
second library. That's why I intentionally returned the exception (throw t;).

Please correct me if my understanding is wrong.


> 
> 5. Improvement A is not addressed.
      A. Treat the case when "fd.exists()" as fatal and rethrow original
exception.
     I throw the original exception back to AprLifeCycleListener.java as
mentioned. 

     In Library.java, i am throwing this and in AprLifecycleListener.java, I
logged this exception as warning. Forgot to mark it as Fatal :)

     throw new LibraryNotFoundError(name, err.toString());     

> 
> 6. Javadoc for LibraryNotFoundError adds no value. Yes, neither does much of
> the Javadoc in that package but that is not a reason to continue in the same
> direction.
> 
> 7. Log messages inr AprLifecycleListener.init should not be made directly to
> the logger. Check the svn history for that file to find out why.

   Sure, I will check. I found only initInfoLogMessages variable in
AprLifecycleListener.java, did not find the Fatal or Warning variable so i felt
no other way than directly print to the log file by log.warn().

> 
> In applying and reviewing the patch I have fixed the various issues so I
> should be committing this fairly soon.
> 
> Don't be concerned about how much has been changed. The first patch I
> offered was unrecognisable by the time Bill had finished with it. Your first
> patch is a lot better than my first patch.


  Do we have any guidelines document so that i can go through and follow
accordingly?

Thanks so much for your time :)

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to