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

Master #7

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Master #7

wants to merge 6 commits into from

Conversation

AshasTob
Copy link

Refactored code structure;
Improved debug functionality;
Removed unused code;
Removed redundany methods;

Restructured code, splitted by classes and namespaces.
Minor functonality losses.
Added possibility to make account of slow connection.
Used config file for example.php
Created logger class,
Added new confing: debug,
Refactroed example.php a bit.
Fixed all authority rights and package names.
Added nice debug trace style.
@AshasTob
Copy link
Author

Dear Voronenko,
Thank you for your lecture, I've found on web. I am very eager to contribute to your project, so I did some code refactoring to code that you presented.

@AshasTob AshasTob closed this Apr 23, 2017
@AshasTob AshasTob reopened this Apr 23, 2017
@Voronenko
Copy link
Owner

@AshasTob Alexander, generally I prefer to keep it in raw php way as it is now, because it demostrates the core steps . Introducing namespaces, autoloaders etc - will make it to look like http://www.smart-jokes.org/programmer-evolution.html :)

There are already plenty of composer packages implementing the same logic. Goal of this repo - is to demonstrate - that you do not need 15+ files 3rd party package to introduce OTP.

@AshasTob
Copy link
Author

@Voronenko Dear Vyacheslav,
Thank you for finding time to review the code.
You are right, I feel very the same about this refactoring :)

The reason I did it is that I wanted to contribute to some small project to better understand how github works. So I wanted to try out how pull requests work and to get my updates installed in real project.

I am actually going soon to try creating my composer package based on your algorithm code for yii2 framework(I know that it already exists tho) just to practice. Hope you are fine with this?

Thank you for review and response. I accept that you can decline this pull request.

@Voronenko
Copy link
Owner

@AshasTob We can make branch , like PSR.
Btw, per PSR - using include is anti pattern, instead you need to use loader, and arrange classes appropriately.

  • feel free to rename my surname to smth more useful like SimpleOTP , etc

@AshasTob
Copy link
Author

@Voronenko Thank you for this opportunity 👍
I will work on this on weekend.

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.

2 participants