rydurham / L4withSentry

Laravel 4 and Sentry 2.0
300 stars 91 forks source link

Refactor into a Laravel Package #55

Closed rossey closed 10 years ago

rossey commented 10 years ago

I've already moved the code into a Laravel package. It needs a little more polish but is this something that would be of use to anyone else? This repository could stay as it is but then include the Service/Controller etc. layer inside the package?

rydurham commented 10 years ago

I am not sure - it seems somewhat unnecessary to me. What would we gain by doing this?

rossey commented 10 years ago

It could primarily help with separating the L4withSentry code from developers' code. It would mean any updates you publish would be then managed by composer. We've already got a Laravel install here so having the ability to install a package that provides administration for Sentry users and groups is a good use case.

sporchia commented 10 years ago

I could see uses to having a drop in Sentry abstraction, updated by composer.

rossey commented 10 years ago

@sporchia it also means that developers' don't have to worry about updating their install of L4withSentry

rydurham commented 10 years ago

You make some good points @rossey. Would you be willing to submit a pull request?

rossey commented 10 years ago

@rydurham thanks Ryan. It might require more than a pull request as your effectively re-factoring the L4withSentry code into a package. Although we may have to consider setting up travis-ci to pull the package into Laravel and run the unit tests :)

J5Dev commented 10 years ago

I would also really like to seer this, this is a fantastic effort, and brilliant as a starting point, which I have used to help me get my head around some of the various aspects of both L4 & Sentry.

Having it as a package, which can be added to my own L4 starting points would be a fantastic way to keep your fantastic Sentry work, whilst having full control of other aspects of L4.

Also I prefer using Foundation, so don't have to tear out Bootstrap each time :)

SamiOmer commented 10 years ago

As a developer who uses Laravel and Sentry 2 I would most definitely use this in every project if it were distributed as a package. Is there any way I can help? Thanks to all the contributors - I'm a real big fan of this repo!!!

J5Dev commented 10 years ago

Incidentally, to follow up my earlier post with a slight off-topic addition, Im happy to put-together/help with a Foundation branch of the the main repo, regardless of whether this goes ahead or not :)

rossey commented 10 years ago

We'll also have to consider how the tests will work as they currently rely on the whole laravel framework being there, so setting up travis-ci or similar might be a good step

rydurham commented 10 years ago

You have convinced me - this sounds like a good idea. However, my original intention for this repo was to serve as a reference for a Sentry implementation, and I would like to keep it as is. To that end, I will start a new repo for the package implementation.

Quick question: Users would be able to use php artisan view:publish to customize the views for a given project, but are there any thoughts about what the published views should look like? Should we try to be UI Framework Agnostic or should we stick with Bootstrap?

I am swamped at the moment, but I will try to get this up and running as soon as I have time.

rossey commented 10 years ago

Hi Ryan, I've already split this into a package, do you want me to push it so you can take a look, and that could then be a starting point?

I think trying to create framework agnostic views could make extra work at the moment. Laravel provides bootstrap components out of the box with for example pagination. After the user has published the views it is then up to them how they modify them.

rossey commented 10 years ago

Hi @rydurham I've pushed the package to https://github.com/wearebase/sentry-manager-laravel-package so hopefully that could be a starting point?

todo:

rydurham commented 10 years ago

Hi @rossey - thank you! I have begun pulling something together. Your repo looks great - an excellent starting point.

rossey commented 10 years ago

Thanks @rydurham no problem! It still needs some work but at least it's a starting point :)

p.s. I didn't know your e-mail address for the composer.json so you'll probably want to update that.

rydurham commented 10 years ago

Ok - take a look at this: https://github.com/rydurham/Sentinel

jimthedev commented 10 years ago

Great work guys. I will be heading over to Sentinel to check it out. This will be super useful when it comes to my app that is based on this starter project. I can't wait to be able to pull down updates instead of grepping for changes. Cheers for all the fantastic work.

rossey commented 10 years ago

@rydurham looks great! :)

jimthedev commented 10 years ago

I'm currently using Sentinel and it works great. I would love to see this project adapted to make use of Sentinel. Are we ready to pull the trigger, should we wait until Sentinel 0.1.2? Are there downsides to the current project name (L4withSentry) or should this example move forward with the current name? My only apprehension would be that Sentinel now has a name and Laravel won't be on v4.x forever. Thoughts?

rossey commented 10 years ago

I think once we have some tests in place with Sentinel we can look at updating this to use it?

jimthedev commented 10 years ago

That sounds fair. Would it be worth having a develop branch on this project so that we can start at least start to prune some of the old code from this project that has been moved into the library?

rydurham commented 10 years ago

I am not against this, @jcummins, however refractoring to make use of Sentinel would essentially just leave a basic app shell and nothing more. While that could be useful, I am not sure if this repo is the best place for that. And as you mention, the name is a bit misleading, and Laravel won't be on V4 forever. To that end, I think it might be good to work on a "Laravel Starter Pack", separate from this repo, that provides a basic shell for development.

I have pulled something together as a starting point, but it needs work: Laravel Starter Pack.

rydurham commented 10 years ago

@J5Dev I would like to include support for foundation, but I am not sure the best way to do it. Creating a separate branch is an option, but that will complicate updating the code base going forward. I am wondering if there might be a more dynamic way to approach this - I will need to ponder this. Any thoughts?

jimthedev commented 10 years ago

@rydurham Makes sense. I would tend to agree that this package is probably not the right place for that basic shell app to live. Laravel Starter Pack seems like a very reasonable description. At the moment it might look bare, but I am sure it will help out those that might be looking for a bare bones Sentinel implementation.

In terms of UI Frameworks and @J5Dev's comments, would it be worth looking at crafting some instructions for people that want to write their own UI packages to extend Sentinel? This would allow others who want those features to maintain that repo as essentially a plugin for Sentinel. At the moment it seems like that is probably quite a while down the road, but at least a plugin would allow those that want to extend functionality to do so.

rydurham commented 10 years ago

I think what I may try to do is create separate folders for each framework's views, and then try to write a custom artisan command for publishing them. This could then allow us to support multiple frameworks (As well as "no" framework) in the same branch. I am going to open an issue for this on Sentinel.

jimthedev commented 10 years ago

Love that idea. Hadn't thought of a custom artisan command.

On Feb 9, 2014, at 4:04 PM, Ryan Durham notifications@github.com wrote:

I think what I may try to do is create separate folders for each framework's views, and then try to write a custom artisan command for publishing them. This could then allow us to support multiple frameworks (As well as "no" framework) in the same branch. I am going to open an issue for this on Sentinel https://github.com/rydurham/Sentinel.

Reply to this email directly or view it on GitHubhttps://github.com/rydurham/L4withSentry/issues/55#issuecomment-34588587 .

rydurham commented 10 years ago

I am going to close this issue. Further discussion should happen on the Sentinel package repo.