-
Notifications
You must be signed in to change notification settings - Fork 1
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
added admin user to migrations and read me, reformatted to PSR-1/PSR-2 #3
base: rewrite
Are you sure you want to change the base?
Conversation
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.
@stosh15x Thank you for your ideas. I've added some comments. Would like to hear from you :)
@@ -16,6 +16,9 @@ The recommended way to install composer packages is: | |||
|
|||
``` | |||
$ composer require bakkerij/cakeadmin:dev-rewrite | |||
$ composer require friendsofcake/crud:^4.3 | |||
$ composer require gourmet/knp-menu:~0.4 | |||
$ composer require holt59/cakephp3-bootstrap-helpers:dev-master |
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.
Dependencies can be removed from documentation and added to our composer.json
right?
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.
When I had them in our dependencies they were not being updated with composer so I moved them into app and it worked. This would be another shell solution.
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.
This is because you added the plugin to your plugins/Bakkerij/CakeAdmin
for development reasons. Composer won't get that file.
When the package is loaded by an user, the package is located in the vendor
folder, and Composer will recognize it, and load its dependencies.
TL;DR: This is only needed for development reasons but it might be solved another way...
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.
Actually I used composer to download your version into vendor. Deleted the cakeadmin directory from vendor/bakkerij and then cloned my fork into the directory. This was the only way I could work on the files without routing issues.
Either way you had the CRUD version in the dependencies of cakeadmin and composer did not pull it in.
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.
I stand corrected when I just started from scratch and the 2 were imported correctly when I used
composer require bakkerij/cakeadmin:dev-rewrite
use Migrations\AbstractMigration; | ||
|
||
class Initial extends AbstractMigration { | ||
|
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.
Do we have a standard according to brackets {
?
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.
I just used NetBeans auto format
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.
CakePHP has its own standard: http://book.cakephp.org/3.0/en/intro/conventions.html
You can validate your code using: https://github.com/cakephp/cakephp-codesniffer
It might be a good idea to follow those standards and conventions... Probably there is a plugin for Netbeans to auto format to CakePHP's conventions, else you should do it manually (http://blog.bobbyallen.me/2013/03/24/configuring-netbeans-for-psr-1-and-psr-2-coding-guidelines/).
'password' => 'test', | ||
'active' => 1 | ||
]); | ||
$AdminTable->save($user); |
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.
Using a seed would be cleaner, but personally I prefer a shell to create an user yourself...
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.
I agree a shell would be the final solution but this can get us up and testing very quickly until the shell has been created
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.
So let's accept this as a quick dirty solution for now :P
Great work @stosh15x, I'll check it out asap! |
I updated the migrations to add a default admin user to the cakeadmin_administrators table. I also updated the readme file to reflect the login credentials.
Edit by @bobmulder:
List of to do's for this PR:
Implementations
Bugfixes