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

Fix logging into parent directory #12

Merged
merged 1 commit into from
Sep 5, 2018
Merged

Fix logging into parent directory #12

merged 1 commit into from
Sep 5, 2018

Conversation

RiKap
Copy link
Contributor

@RiKap RiKap commented Aug 31, 2018

Hey.
There is a bug, when it create log file in parent directory of $this->directory

  • It will skip dot directories, you should ignore them
  • Fix bug when $this->directory does not end with directory separator.

How to reproduce?:

  1. Set your logDir for TracyLogging as %appDir%/../log
  2. Set modo to production
  3. Put somewhere in presenter action throw new \Nette\InvalidArgumentException('TEST').
  4. On first throw .html will be in log directory, but second .html, of the same exception, will be in parent of log dir.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 44.444% when pulling c35d2f1 on RiKap:patch-1 into a79d14d on contributte:master.

@RiKap
Copy link
Contributor Author

RiKap commented Sep 5, 2018

@f3l1x What do you think?

@f3l1x
Copy link
Member

f3l1x commented Sep 5, 2018

Hi. Looks good to me. I still dont know how is that possible. I have copied these lines from Tracy.

@RiKap
Copy link
Contributor Author

RiKap commented Sep 5, 2018

@f3l1x See this line https://api.nette.org/2.4/source-Tracy.Logger.php.html#137 Before every log, Tracy add / after $this->directory. This line is missing right now in AbstractLogger. :)

Can you please check #11 and if you approve both PR, release new version. :) Thank you.

@f3l1x f3l1x merged commit 67a02b3 into contributte:master Sep 5, 2018
@f3l1x
Copy link
Member

f3l1x commented Sep 5, 2018

I see. Thanks.

@RiKap RiKap deleted the patch-1 branch September 5, 2018 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants