Skip to content

Commit

Permalink
formatting
Browse files Browse the repository at this point in the history
  • Loading branch information
taylorotwell committed Dec 27, 2019
1 parent c222f6d commit 0bec06c
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 43 deletions.
2 changes: 1 addition & 1 deletion app/Http/Kernel.php
Expand Up @@ -15,7 +15,7 @@ class Kernel extends HttpKernel
*/
protected $middleware = [
\App\Http\Middleware\TrustProxies::class,
\App\Http\Middleware\HandleCors::class,
\Fruitcake\Cors\HandleCors::class,
\App\Http\Middleware\CheckForMaintenanceMode::class,
\Illuminate\Foundation\Http\Middleware\ValidatePostSize::class,
\App\Http\Middleware\TrimStrings::class,
Expand Down
9 changes: 0 additions & 9 deletions app/Http/Middleware/HandleCors.php

This file was deleted.

40 changes: 7 additions & 33 deletions config/cors.php
Expand Up @@ -4,57 +4,31 @@

/*
|--------------------------------------------------------------------------
| Laravel CORS Options
| Cross-Origin Resource Sharing (CORS) Configuration
|--------------------------------------------------------------------------
|
| The allowed_methods and allowed_headers options are case-insensitive.
| Here you may configure your settings for cross-origin resource sharing
| or "CORS". This determines what cross-origin operations may execute
| in web browsers. You are free to adjust these settings as needed.
|
| You don't need to provide both allowed_origins and allowed_origins_patterns.
| If one of the strings passed matches, it is considered a valid origin.
|
| If array('*') is provided to allowed_methods, allowed_origins or allowed_headers
| all methods / origins / headers are allowed.
| To learn more: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS
|
*/

/*
* You can enable CORS for 1 or multiple paths.
* Example: ['api/*']
*/
'paths' => [],
'paths' => ['api/*'],

/*
* Matches the request method. `[*]` allows all methods.
*/
'allowed_methods' => ['*'],

/*
* Matches the request origin. `[*]` allows all origins.
*/
'allowed_origins' => ['*'],

/*
* Matches the request origin with, similar to `Request::is()`
*/
'allowed_origins_patterns' => [],

/*
* Sets the Access-Control-Allow-Headers response header. `[*]` allows all headers.
*/
'allowed_headers' => ['*'],

/*
* Sets the Access-Control-Expose-Headers response header.
*/
'exposed_headers' => false,

/*
* Sets the Access-Control-Max-Age response header.
*/
'max_age' => false,

/*
* Sets the Access-Control-Allow-Credentials header.
*/
'supports_credentials' => false,

];

6 comments on commit 0bec06c

@Rah1x
Copy link

@Rah1x Rah1x commented on 0bec06c Sep 21, 2020

Choose a reason for hiding this comment

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

Why would you remove all these defination comments? I just upgraded to 7x and now all of a sudden ive this new config file and no idea what going on! The upgrade guide only barely just mentions it.

perhaps you could please put the link in comments to refer to the options?
https://github.com/fruitcake/laravel-cors

@driesvints
Copy link
Member

Choose a reason for hiding this comment

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

@Rah1x you own this file. Feel free to re-add them.

@mfn
Copy link
Contributor

@mfn mfn commented on 0bec06c Sep 21, 2020

Choose a reason for hiding this comment

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

That's IMHO besides the point, I'm with @Rah1x and would prefer to keep them.

Here's, at least my, reasoning:

  • those comments are useful
  • if you've long living Laravel based projects, you eventually want to sync these configs to be aware of changed/added ones
    But this clashes a bit here, even if I add them back, any feature attempt to sync will show as changed

I've done this successfully on projects starting with 5.1 and now being on L8 and usually incrementally changes and slight deviations are easy to handle (and usually only have to be done twice a year, when upgrades happen, so it's "manageable").

There's really no technical merit for removing any of this, and to put it into the words of Taylor: those comments and the ones from the other config files are just beautiful! (and useful)

@driesvints
Copy link
Member

Choose a reason for hiding this comment

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

We want to keep a consistent code styling across our files, sorry.

@mfn
Copy link
Contributor

@mfn mfn commented on 0bec06c Sep 21, 2020

Choose a reason for hiding this comment

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

So you're saying, would the comments have been like this they could have stayed?
image

@driesvints
Copy link
Member

Choose a reason for hiding this comment

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

We've already updated the comments to our new styling and probably won't be changing anything here sorry. If you want the old style feel free to copy them from the fruitcake repo.

Please sign in to comment.