Skip to content

Conversation

@benaclejames
Copy link
Contributor

Reverting to refactor log deletion functionality. We should be using File.Delete and ensuring we catch any IO exceptions for the not-so-unlikely case where SRanipal Runtime still has a lock on it

@benaclejames benaclejames requested a review from regzo2 January 12, 2024 21:07
@benaclejames benaclejames marked this pull request as draft January 12, 2024 21:08
@regzo2
Copy link
Contributor

regzo2 commented Jan 12, 2024

This change does not delete the logs, it clears the existing content within the stream to avoid dealing with locking apps. Are you saying you want to use File.Delete instead because it could just be changed to work with that (with the downside of not deleting the logs while SRanipal is already running)?

@VirtualMarten
Copy link
Contributor

I believe the main issue is exception handling. For example what if we don't have permission to access the log files.
Because this is run on init and isn't essential to the function of the module, it'll probably be best to just catch any exceptions and log a warning.

@benaclejames
Copy link
Contributor Author

File.Delete essentially does the same internally. Makes more sense to just use the built in functionality instead System.IO instead of re-implementing it. As for code readibility, File.Delete also makes intention far clearer

@VirtualMarten
Copy link
Contributor

I think @regzo2 solution makes more sense here because we can't delete a file SRanipal currently has a lock on.

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.

4 participants