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.

Sign up for GitHub

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

Jump to bottom

[5.2] Add Bootstrap Composer scripts #12827

Merged
merged 1 commit into from
Mar 23, 2016
Merged

[5.2] Add Bootstrap Composer scripts #12827

merged 1 commit into from
Mar 23, 2016
+42 −0

Conversation

barryvdh
Copy link
Contributor

Alternative to laravel/laravel#3695 and the manual scripts

Instead of running a php artisan command, you can run it like this:

"scripts": {
        "post-install-cmd": [
            "Illuminate\\Bootstrap\\Composer::postInstall"
        ],
        "post-update-cmd": [
            "Illuminate\\Bootstrap\\Composer::postUpdate"
        ]       
    }

This avoids loading the compiled path or needing any bootstrap configuration.
Also adds more flexibility when upgrading, instead of a laravel/laravel command.

I now used postInstall and postUpdate to call clearCompiled(). We could also call clearCompiled directly, but this way you could perhaps add different functions later on when needed.

@barryvdh barryvdh mentioned this pull request Mar 23, 2016
Closed
@GrahamCampbell GrahamCampbell changed the title Add Bootstrap Composer scripts [5.2] Add Bootstrap Composer scripts Mar 23, 2016
@GrahamCampbell
Copy link
Member

Maybe this needs sending to 5.1 @taylorotwell?

@taylorotwell taylorotwell merged commit 8e62cda into laravel:5.2 Mar 23, 2016
@taylorotwell
Copy link
Member

Moved to Illuminate\Foundation\ComposerScripts. Thanks... feel free to submit PR to laravel/laravel

@barryvdh barryvdh deleted the patch-8 branch March 23, 2016 14:18
GrahamCampbell
GrahamCampbell reviewed Mar 23, 2016
View reviewed changes
src/Illuminate/Bootstrap/Composer.php
@@ -0,0 +1,42 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no?

@vlakoff
Copy link
Contributor

vlakoff commented Mar 23, 2016

👍 Looks great, thanks barryvdh.

And yes this FIX (all installations being BROKEN currently) should go into Laravel 5.1 LONG TERM SUPPORT as well.

@barryvdh
Copy link
Contributor Author

It wouldn't hurt to add this to L5.1, so it will work consistently in 5.1+ with the same composer.json scripts.

This was referenced Mar 24, 2016
Closed
Merged
Merged
@completegibbon
Copy link

This doesn't appear to have been added into 5.1 LTS - I am still unable to do a fresh install.

@vlakoff
Copy link
Contributor

vlakoff commented Apr 4, 2016

Currently, it can fail only if there is a stale compiled cache. And I precisely encountered an error because of this, so indeed that can happen.

@vlakoff vlakoff mentioned this pull request Apr 5, 2016
Closed
@GrahamCampbell
Copy link
Member

I'll talk to Taylor again today regarding backporting this to 5.1.

@LKaemmerling
Copy link

@GrahamCampbell it is really importend to fix this issue as soon as possible. Deployments only work with an really old composer version.

@barryvdh
Copy link
Contributor Author

barryvdh commented Apr 8, 2016

It is backported: laravel/laravel#3738

@LKaemmerling
Copy link

but not tagged at this moment right?

@GrahamCampbell
Copy link
Member

yes, it is tagged

@LKaemmerling
Copy link

mh crazy, i have the actual LTS Version, the changes mergend into the composer.json, deleted composer.lock and got still the same error

@barryvdh barryvdh mentioned this pull request Apr 23, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@barryvdh @GrahamCampbell @taylorotwell @vlakoff @completegibbon @LKaemmerling @kevinsimard