https://issues.apache.org/bugzilla/show_bug.cgi?id=57021
--- Comment #17 from VIN <reachme.va...@gmail.com> --- (In reply to VIN from comment #16) > (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 :) I have gone through the fixed code. It is nice and simple. One query is: In AprLifecycleListener.java, the LibraryNotFound exception is added to initInfiLogMessages variable. I suspect it would print the error as INFO. -- 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
