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

PHP 8.0 support 🚀 and Laravel 6, 7 and 8 support #342

Merged
merged 3 commits into from
Dec 1, 2020

Conversation

tortuetorche
Copy link
Contributor

  • Add PHP 7.3, 7.4 and 8.0 support
  • Add Laravel 6, 7 and 8 support
  • Update phpspec tests
  • Drop PHP 5.6, 7.0 and 7.1 support
  • Drop Laravel 5 support

* Add PHP 7.3, 7.4 and 8.0 support
* Add Laravel 6, 7 and 8 support
* Update phpspec tests
* Drop PHP 5.6, 7.0 and 7.1 support
* Drop Laravel 5 support
Copy link
Collaborator

@PatrickRose PatrickRose left a comment

Choose a reason for hiding this comment

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

All looks good - though does #320 still apply?

- 7.2
- 7.3
- 7.4
- 8.0snapshot
Copy link
Collaborator

Choose a reason for hiding this comment

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

Surprised that Travis doesn't have a proper PHP8 thing already!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Last time I checked, they don't! But soon I hope 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PatrickRose
Copy link
Collaborator

In fact - there's a lot of tickets about incompatabilities with laravelcollective/html - how many of them still apply?

@tortuetorche
Copy link
Contributor Author

All looks good - though does #320 still apply?

Good catch, I think we can remove the getToken() support and just keep token()

Reference: https://laravel.com/docs/5.4/upgrade

@tortuetorche
Copy link
Contributor Author

FYI build (from my fork) passed: https://travis-ci.com/github/tortuetorche/bootstrapper/builds/205628104

By the way, you should migrate from the legacy https://travis-ci.org to the new https://travis-ci.com, to get faster builds 👍
I did it for my repositories, it's like night and day 😆

@tortuetorche
Copy link
Contributor Author

In fact - there's a lot of tickets about incompatabilities with laravelcollective/html - how many of them still apply?

I need to do more manual tests to answer you

@tortuetorche
Copy link
Contributor Author

All looks good - though does #320 still apply?

Done!

Build (from my fork) passed: https://travis-ci.com/github/tortuetorche/bootstrapper/builds/205632538

@PatrickRose
Copy link
Collaborator

By the way, you should migrate from the legacy https://travis-ci.org to the new https://travis-ci.com, to get faster builds 👍

I'd love to, but it looks like because this is still under @patricktalmadge's name it's something they'd have to do. With the amount of work that gets done on Bootstrapper on a daily basis (I no longer use Laravel on a day to day basis), it's probably not worth it :(

@tortuetorche
Copy link
Contributor Author

tortuetorche commented Dec 1, 2020

In fact - there's a lot of tickets about incompatabilities with laravelcollective/html - how many of them still apply?

I need to do more manual tests to answer you

Good news, I tested the Form::select(), Form::submit(), Alert, Button and Icon components, in a real Laravel 6 application and PHP 8.0, without any problems!

@tortuetorche
Copy link
Contributor Author

@PatrickRose
Copy link
Collaborator

Awesome, I'll merge this later tonight and tag it for a release.

Thanks so much for your contribution!

@tortuetorche
Copy link
Contributor Author

@PatrickRose You've have done much much more than me on this package, so thanks to you!

Awesome, I'll merge this later tonight and tag it for a release.

Thanks so much for your contribution!

You've have done much much more than me on this package, so big thank to you!

@PatrickRose PatrickRose merged commit b9bb145 into patricktalmadge:master Dec 1, 2020
@PatrickRose
Copy link
Collaborator

And tagged - thanks again!

@tortuetorche
Copy link
Contributor Author

And tagged - thanks again!

FYI, the 5.12.0 tag isn't present in Packagist (https://packagist.org/packages/patricktalmadge/bootstrapper)
Maybe you need to force update on the Packagist side?
As a workaround, I use this in my composer.json file:

    "require": {
        //...
        "patricktalmadge/bootstrapper": "dev-master",
        //...
    },

@tortuetorche
Copy link
Contributor Author

tortuetorche commented Dec 3, 2020

FYI, the 5.12.0 tag isn't present in Packagist
Maybe you need to force update on the Packagist side?

@PatrickRose (or @Anahkiasen) I think you can do a manual update on https://packagist.org/packages/patricktalmadge/bootstrapper

If you login with your GitHub account, then you can click on the Update green button, just under the package's title.
I did it for a package who I don't have administrator permission 👍

Here a screenshot:
packagist

@PatrickRose
Copy link
Collaborator

I've done the update, sorry about that!

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.

2 participants