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

4.x Remove App Settings #2589

Merged
merged 3 commits into from Mar 1, 2019

Conversation

Projects
None yet
4 participants
@l0gicgate
Copy link
Contributor

commented Feb 23, 2019

Remove App settings and associated tests as per discussion @akrabat via Slack.

We have removed all of the settings from the App. The customization that those settings provided are now available in the instantiation of core middleware or the logic to apply those settings has changed.

The settings deprecated:

  • httpVersion This can be set via your PSR-7 implementation's ResponseFactory if available
  • routerCacheFile You have to get the router via App::getRouter() then $router->setCacheFile($file)
  • displayErrorDetails is now moved to ErrorMiddleware's constructor parameters
  • outputBuffering is now moved to OutputBufferingMiddleware's constructor parameters
  • determineRouteBeforeAppMiddleware is now moved to order in which you add RoutingMiddleware
  • responseChunkSize is moved to ResponseEmitter's constructor parameters
  • addContentLengthHeader is defined by whether or not you add ContentLengthMiddleware to your middleware queue

Will need to update CHANGELOG

@l0gicgate l0gicgate added the Slim 4 label Feb 23, 2019

@l0gicgate l0gicgate added this to the 4.0 milestone Feb 23, 2019

@coveralls

This comment has been minimized.

Copy link

commented Feb 23, 2019

Coverage Status

Coverage decreased (-0.009%) to 99.41% when pulling 12ff350 on l0gicgate:4.x-RemoveSettings into 5311e95 on slimphp:4.x.

@froschdesign

This comment has been minimized.

Copy link

commented Feb 23, 2019

@l0gicgate

…as per discussion @akrabat

Unfortunately, this information is useless because nobody knows where to find this discussion. It would be better to publish a summary and the result of the discussion. A link reference would also be sufficient.

@l0gicgate

This comment has been minimized.

Copy link
Contributor Author

commented Feb 23, 2019

would be better to publish a summary and the result of the discussion. A link reference would also be sufficient.

Fair point, it's just that not all discussions happen via the GitHub repository. We have a private channel on Slack where we have some internal discussions.

I do agree though that it should maybe be discussed in issues instead of on our private channel. I apologize for that.

@akrabat

This comment has been minimized.

Copy link
Member

commented Feb 23, 2019

@l0gicgate

This comment has been minimized.

Copy link
Contributor Author

commented Feb 23, 2019

@akrabat @froschdesign I just added the details in the description of this PR as well.

@froschdesign

This comment has been minimized.

Copy link

commented Feb 23, 2019

@l0gicgate @akrabat
Please do not misunderstand me, this is only a hint that referring to a (private) discussion without details or link in a public issue tracker does not work.

@l0gicgate

This comment has been minimized.

Copy link
Contributor Author

commented Feb 23, 2019

@l0gicgate @akrabat
Please do not misunderstand me, this is only a hint that referring to a (private) discussion without details or link in a public issue tracker does not work.

I completely agree, I will make sure going forward that everything is documented here as much as possible if we do have internal discussions. I normally open issues so we can track it, in this instance I did not and I should have. My apologies.

@akrabat

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

Will need to update CHANGELOG

Please update the CHANGELOG :)

@l0gicgate

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2019

Please update the CHANGELOG :)

Done @akrabat

@akrabat

akrabat approved these changes Mar 1, 2019

akrabat added a commit to akrabat/Slim that referenced this pull request Mar 1, 2019

@akrabat akrabat merged commit 12ff350 into slimphp:4.x Mar 1, 2019

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.009%) to 99.41%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@l0gicgate l0gicgate deleted the l0gicgate:4.x-RemoveSettings branch Mar 2, 2019

@l0gicgate l0gicgate referenced this pull request Apr 25, 2019

Merged

Slim 4 Alpha Release #2665

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.