-
Notifications
You must be signed in to change notification settings - Fork 2
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
Single responsibility and new binary script #1
base: master
Are you sure you want to change the base?
Conversation
Clase de instalador aislada para un mejor control.
@Faryshta plz reply and make review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Chofoteddy I dont understand why you are changing the class functionality to a binary file since this is an skeleton, not a library so it wont be installed as library.
"classmap": [ | ||
"ComposerListener.php" | ||
] | ||
}, | ||
"bin": ["codecept-skeleton"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why put this as a binary file? it wont be executed on third party repos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Chofoteddy answer back asap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason: is to be able to use it as both (library or binary). The need arose because I wanted to use the functionality offered by the library, but I could not execute it from composer unless it was binary. Example: Codeception has its library and binary at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deployer is only meant to be used in the skeleton, it wont work as a library.
So I dont understand how you intend to use the binary.
Codeception is a library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, Codeception is a library, with a binary that supports the construction of the initial skeleton: the same purpose as this pull-request.
https://github.com/Codeception/Codeception/blob/2.4/composer.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok it seems the use case for bin files is not clear.
When someone adds a dependency on a project the 'require' section of composer.json its called a library, said library can include bin files to be executed by the project.
This repository is not a library and is never meant to be included as dependency in the 'require' section of any project. This repository is meant to be used as the skeleton for a project. So there is no use for any bin file here.
In order to have a better control over the drivers and be able to add functionality for each one more easily, I have separated the code, following the methodology of single responsibility.
I have also added a new file that will be executed as a binary to be able to execute it from composer in any project.
I attach an image to summarize the changes made.