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

Drupal 7 & 8 Driver #48

Merged
merged 4 commits into from
Jul 26, 2016
Merged

Drupal 7 & 8 Driver #48

merged 4 commits into from
Jul 26, 2016

Conversation

aka-tpayne
Copy link
Contributor

This driver adds support for Drupal 7 and 8.

@taylorotwell
Copy link
Member

How does this handle the situations when Drupal does things like /update.php/more/path/here?

@aka-tpayne
Copy link
Contributor Author

@taylorotwell I've just rewritten the front controller code to handle this. Also while doing so, I found that this method also covered for some of the previous checks and those have since been removed.

@aka-tpayne
Copy link
Contributor Author

@taylorotwell is there something I can do to help move this along?

@McGo McGo mentioned this pull request May 27, 2016
@barryvdh
Copy link

Perhaps fix the merge conflicts? ;)

@jaymiejones86
Copy link

@Cybnext anything I can do to help get this PR back to green so it can hopefully get merged soon?

@aka-tpayne
Copy link
Contributor Author

Alright, back to green, hopefully we can get this merged now :)

@dustinleblanc
Copy link

@Cybnext I was just about to do a version of this that's compatible with drupal-composer/drupal-project (docroot is nested at /web/). Would it make sense to just add the path logic into the checks here? All the conditionals are getting silly, but this structure is gaining popularity in the Drupal-verse.

@cferthorney
Copy link

Given Drupal has launched alpha support for it's own packagist/composer I'd suggest composer support is a must. It may as well default to web/ as the regex can always be updated by another PR if necessary. I can't be sure if drupal.org will suggest an alternative location for things (IE over web/) or not though.

@thijsvdanker
Copy link

Given Drupal has launched alpha support for it's own packagist/composer I'd suggest composer support is a must.

I think this should just get merged and a new PR for the (alpha) composer support would be more appropriate.

@jaymiejones86
Copy link

I would have to agree. Iterate on the basic implementation.
On Tue, 28 Jun 2016 at 6:15 PM, Thijs van den Anker <
notifications@github.com> wrote:

Given Drupal has launched alpha support for it's own packagist/composer
I'd suggest composer support is a must.

I think this should just get merged and a new PR for the (alpha) composer
support would be more appropriate.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#48 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABU36ViIfeCq_YDSDCpl3gFcziV8zYuhks5qQNgJgaJpZM4Iaj-z
.

Jaymie Jones
@pixelstackcom
http://jaymiejonesphoto.com
http://codercatchup.com
https://devbase.io

@aka-tpayne
Copy link
Contributor Author

I would agree with getting the basic pulled into master. Hopefully that can happen soon, kind of frustrating to have this sitting green for another month

@McGo
Copy link

McGo commented Jul 26, 2016

Any progress on this? Is there anything that has to be done before merging into master?

@aka-tpayne
Copy link
Contributor Author

@McGo Nope, nothing we can do at this point. It's been ready for awhile now and could easily be merged into master, just have to wait for a maintainer to do it, I guess.

@taylorotwell taylorotwell merged commit 58e3c07 into laravel:master Jul 26, 2016
@taylorotwell
Copy link
Member

There ya go.

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.

None yet

8 participants