You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "gdziadkiewicz (via GitHub)" <gi...@apache.org> on 2025/06/15 20:19:53 UTC

[PR] Add draft of tests for file rolling regression (logging-log4net)

gdziadkiewicz opened a new pull request, #257:
URL: https://github.com/apache/logging-log4net/pull/257

   Fixes #250 
   
   TODO


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add tests for regression in file rolling and fix bug (logging-log4net)

Posted by "Flohack74 (via GitHub)" <gi...@apache.org>.
Flohack74 commented on code in PR #257:
URL: https://github.com/apache/logging-log4net/pull/257#discussion_r2164727735


##########
src/log4net/Appender/RollingFileAppender.cs:
##########
@@ -681,18 +681,16 @@ protected List<string> GetExistingFiles(string baseFilePath)
     using (SecurityContext?.Impersonate(this))
     {
       string fullPath = Path.GetFullPath(baseFilePath);
-      string dir = Path.GetDirectoryName(baseFilePath);
 
       directory = Path.GetDirectoryName(fullPath);
       if (Directory.Exists(directory))
       {
         string baseFileName = Path.GetFileName(fullPath);
-
+        

Review Comment:
   You might want to clean that whitespace change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add tests for regression in file rolling and fix bug (logging-log4net)

Posted by "gdziadkiewicz (via GitHub)" <gi...@apache.org>.
gdziadkiewicz commented on PR #257:
URL: https://github.com/apache/logging-log4net/pull/257#issuecomment-3028466166

   @FreeAndNil can you take a look? This is review-ready :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add tests for regression in file rolling and fix bug (logging-log4net)

Posted by "gdziadkiewicz (via GitHub)" <gi...@apache.org>.
gdziadkiewicz commented on code in PR #257:
URL: https://github.com/apache/logging-log4net/pull/257#discussion_r2165962573


##########
src/log4net/Appender/RollingFileAppender.cs:
##########
@@ -681,18 +681,16 @@ protected List<string> GetExistingFiles(string baseFilePath)
     using (SecurityContext?.Impersonate(this))
     {
       string fullPath = Path.GetFullPath(baseFilePath);
-      string dir = Path.GetDirectoryName(baseFilePath);
 
       directory = Path.GetDirectoryName(fullPath);
       if (Directory.Exists(directory))
       {
         string baseFileName = Path.GetFileName(fullPath);
-
+        

Review Comment:
   Will do, thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add tests for regression in file rolling and fix bug (logging-log4net)

Posted by "gdziadkiewicz (via GitHub)" <gi...@apache.org>.
gdziadkiewicz commented on code in PR #257:
URL: https://github.com/apache/logging-log4net/pull/257#discussion_r2165975290


##########
src/log4net/Appender/RollingFileAppender.cs:
##########
@@ -681,18 +681,16 @@ protected List<string> GetExistingFiles(string baseFilePath)
     using (SecurityContext?.Impersonate(this))
     {
       string fullPath = Path.GetFullPath(baseFilePath);
-      string dir = Path.GetDirectoryName(baseFilePath);
 
       directory = Path.GetDirectoryName(fullPath);
       if (Directory.Exists(directory))
       {
         string baseFileName = Path.GetFileName(fullPath);
-
+        

Review Comment:
   Done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add tests for regression in file rolling and fix bug (logging-log4net)

Posted by "gdziadkiewicz (via GitHub)" <gi...@apache.org>.
gdziadkiewicz commented on PR #257:
URL: https://github.com/apache/logging-log4net/pull/257#issuecomment-3017954903

   I expect it to be review-ready today.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add tests for regression in file rolling and fix bug (logging-log4net)

Posted by "FreeAndNil (via GitHub)" <gi...@apache.org>.
FreeAndNil merged PR #257:
URL: https://github.com/apache/logging-log4net/pull/257


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org