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

Add use LaratrustUserTrait with a command call #3

Closed
KKSzymanowski opened this issue Jun 12, 2016 · 7 comments
Closed

Add use LaratrustUserTrait with a command call #3

KKSzymanowski opened this issue Jun 12, 2016 · 7 comments

Comments

@KKSzymanowski
Copy link
Collaborator

KKSzymanowski commented Jun 12, 2016

Hi.

I'm currently working on package which:

  1. Reads PHP file
  2. Adds trait import above the class
  3. Adds use Trait at the top of the class definition

Are you interested in integrating it with your package to automatically add LaratrustUserTrait to App\User class(eg. with a command)? If so, I will put together a PR and post it within few days.

@KKSzymanowski
Copy link
Collaborator Author

Pushed it onto Github: https://github.com/KKSzymanowski/Traitor

@KKSzymanowski KKSzymanowski changed the title Automatically add use LaratrustUserTrait Add use LaratrustUserTrait with a command call Jun 13, 2016
@Mathius17
Copy link
Contributor

Hi @KKSzymanowski! Nice idea and thanks for your proposal but to be honest, we are trying to make the package as self-contained as possible. The less things the package does, the better.

@KKSzymanowski
Copy link
Collaborator Author

The less things the package does, the better
Take Laravel/Framework for example. It does a lot of things you may never use but it's quality comes from getting you started quickly and providing virtually all of the needed functionality out of the box.

Nevertheless I can see what drove your decision, but in my opinion, this package(as well as Zizaco/Entrust) has a quite complicated installation process and it's easy to forget one of the steps. I'm not saying it's a drawback, but I think it can be improved greatly.

What I mean is to add these commands:

  • laratrust:make-role - Creates a Role model
  • laratrust:make-permission - Creates a Permission model
  • laratrust:make-models - Calls 2 former commands and adds a LaratrustUserTrait to User model

What I've implemented adds 3 new classes for each command and 2 stubs for Role and Permission model. I don't think, it's that much of a weight. Furthermore, these classes are only used when running commands from console.
In my opinion they significantly speed up the setup process(which is what we want in RAD), but it's of course your opinion, that will matter in the end.

@santigarcor
Copy link
Owner

@KKSzymanowski You're totally right, could you please make the PR with the changes that you're mentioning, so we can give it a try.

Yes, the installation process is too complicated, and we want to improve it to make it as easy as possible, maybe doing everything with a single command would improve that.

@KKSzymanowski
Copy link
Collaborator Author

KKSzymanowski commented Jun 13, 2016

I'm happy to hear that.
Give me a few days, I'm in quite a rush due to the end of semester.

@KKSzymanowski
Copy link
Collaborator Author

One last thought:
What do you think about one more command called laratrust:setup that run laratrust:migration and then laratrust:make-models which is, I think, all you need to do(apart from adding a ServiceProvider) to setup Laratrust.

@Mathius17
Copy link
Contributor

Mathius17 commented Jun 13, 2016

That's exactly what we want! A single command for the entire setup (apart from the ServiceProvider)

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

No branches or pull requests

3 participants