Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'(doc)' make the method RollingFileAppender.RollFile() virtual. #46

Closed
wants to merge 1 commit into from
Closed

Conversation

qub1n
Copy link

@qub1n qub1n commented Jan 29, 2019

I need to post process rolled file by archiver and I can hardly achieve it without making this method virtual.

I need to post process rolled file by archiver and I can hardly achieve it without making this method virtual.
@dpsenner
Copy link
Member

Would it be more appropriate to add an invocation of an emty but virtual PostRollFile method?

@qub1n
Copy link
Author

qub1n commented Jan 29, 2019

Yes following empty method would be sufficient.
protected virtual void OnFileRolled(string filename){}

@dpsenner
Copy link
Member

I propose to add both oldFile and newFile to the method arguments. Is there a reasoning why the pull request is not based onto develop?

@qub1n
Copy link
Author

qub1n commented Jan 29, 2019

Is there a reasoning why the pull request is not based onto develop?
-> No, it was just the default option on GitHub, I will change it.

OK, please cancel this pull request and I will create a new Pull Request on branch 'develop' with following method signature:

protected virtual void OnFileRolled(string fromFile, string toFile){}

@qub1n qub1n closed this Jan 31, 2019
@qub1n
Copy link
Author

qub1n commented Jan 31, 2019

I made new pull request (47) on proper branch, please close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants