Skip to content
This repository was archived by the owner on Dec 14, 2021. It is now read-only.

Allow changing the log file path in preferences#900

Merged
mlangkabel merged 7 commits intoCoatiSoftware:masterfrom
Waqar144:custom-log-dir
Feb 3, 2020
Merged

Allow changing the log file path in preferences#900
mlangkabel merged 7 commits intoCoatiSoftware:masterfrom
Waqar144:custom-log-dir

Conversation

@Waqar144
Copy link
Contributor

Copy link
Contributor

@mlangkabel mlangkabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for handing in this pull request! The code is looking good already, but please fix the things I mentioned in my review.

Apart from that, it would be great if the log file path of the logger is set if the user saves and closes the application settings. Otherwise the settings do not match what really happens.

@Waqar144
Copy link
Contributor Author

Applied requested changes:

  • Directory path for FileLogger is now set in Application::loadSettings() instead of app/main.cpp
  • Fixed tabs
  • Main menu -> 'Show Log Folder' was showing incorrect path, that has been fixed.
  • New log directory path will come into effect immediately on saving prefrences

Copy link
Contributor

@mlangkabel mlangkabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your changes. I left some comments where the code formatting was still off.

I also tested the changes on my system and it is not working. Did you test this? Is it working for you? Does Sourcetrail write the log files to the new location after you change the log directory in the preferences?

@Waqar144
Copy link
Contributor Author

Updated.

  • Fixed formatting
  • Fixed log file not being saved

@mlangkabel
Copy link
Contributor

Looking good now. Thanks again for implementing this feature!

@mlangkabel mlangkabel merged commit f7923eb into CoatiSoftware:master Feb 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants