Skip to content
This repository has been archived by the owner on Jul 12, 2020. It is now read-only.

New PSR-3 compatible Logger #248

Merged
merged 8 commits into from
Aug 28, 2017
Merged

New PSR-3 compatible Logger #248

merged 8 commits into from
Aug 28, 2017

Conversation

Art4
Copy link
Collaborator

@Art4 Art4 commented Aug 22, 2017

This PR introduces a new PSR-3 compatible Logger. It should be used to log all kind of stuff like warnings, errors or debug data.

I've added the Logger already in the SignatureDecipher, but it should used everywhere. The file Deciphers.log can be deleted. Logs are stored in the new /logs folder and are only created if debug in the config is set to true.

refs #221

@Art4 Art4 requested a review from jeckman August 22, 2017 22:31
@Art4 Art4 changed the title New Psr-3 compatible Logger New PSR-3 compatible Logger Aug 22, 2017
@Art4 Art4 mentioned this pull request Aug 22, 2017
12 tasks
@StefansArya
Copy link
Contributor

This is getting more messy for non IDE programmer..
Btw what IDE are you using for making this? @Art4

@Art4
Copy link
Collaborator Author

Art4 commented Aug 23, 2017

This is getting more messy for non IDE programmer..

I don't think so. In opposite is my work based on the best practices and standard recommendations for PHP.

Our problem with this project at the moment (that I'm trying to fix with the refactoring) is the messy code. This PR in special allows us to catch all debug information from the decipher process (and in future from the whole app) in one single place (the /logs folder atm), without the need to use the output buffer.

The logger is based on PSR-3 so if you know it you will know how to use the logger implementation in this PR. In fact I've just copy and pasted the PSR-3 LoggerInterface and other classes for the Logger interface. The reason why I don't simply use the Psr package is that our project goal is to use no dependencies in production, so I can't use composer.

The logger is set to the classes following the inversion of control principle through an argument or using the LoggerAware interface. This leads to a better decoupling of the code parts. In fact you can change the logger, cache or container implementations without touching the classes that uses them. Thanks to the interfaces. "Program to interfaces, not implementations."

Btw: I'm planning to create a new repository where I replace our own implementations with solid 3rd party implementations like Monolog, Pimple and PHPCache (thanks to the PSRs) so this project can be used as a framework agnostic library in every PHP project.

Btw what IDE are you using for making this?

I don't use an IDE. 😉 I'm using editors like Atom, gedit, vim or Notepad++, depends on the OS I'm working on. And a terminal of course.

@StefansArya
Copy link
Contributor

Hmm I see, looks like we have different idea 😅..
For me, a script library will be better if it's looks simple and have faster execution process.

@Art4
Copy link
Collaborator Author

Art4 commented Aug 23, 2017

For me, a script library will be better if it's looks simple and have faster execution process.

You are right. That's why this project called a library in the goals, not a script. 🙂 If your script and/or community grows it is imho important to create structures to maintain the code and allow more features that the community asks for.

@Art4
Copy link
Collaborator Author

Art4 commented Aug 24, 2017

I would be glad if someone could review my pull request. I have the permission to merge this on my own, but I would be happy if someone of the community could take a look first since @jeckman has not much time.

@Art4 Art4 merged commit 1be57dc into jeckman:master Aug 28, 2017
@BelleNottelling
Copy link
Contributor

@Art4 wish I could have reviewed it, but I'm not very good at PHP and honestly I haven't been reading the code. I'm sure you did great though! 😁

@Art4
Copy link
Collaborator Author

Art4 commented Aug 28, 2017

@BenNottelling No problem, everyone does what he can. 🙂 I hope that my code can be useful for someone. Not only as the downloader itself but also for learning.

For me it's important that @jeckman and the community stands behind my work for this project. So I appreciate every question or comment on my PRs, so thanks to you and @StefansArya. 😊

If nobody has anything against my PRs I will merge them after some days.

@Art4 Art4 deleted the Logger branch October 10, 2017 08:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants