Skip to content

BC breaking proposal: move constants to namespace #12

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

Closed
wants to merge 2 commits into from
Closed

BC breaking proposal: move constants to namespace #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a643025
move constants to namespace
stefanotorresi Oct 30, 2018
390843b
remove implements from examples
stefanotorresi Oct 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
"psr/http-message": "The package containing the PSR-7 interfaces"
},
"autoload": {
"psr-4": {
"Fig\\Http\\Message\\": "src/"
}
"files": [
"constants.php"
]
},
"extra": {
"branch-alias": {
Expand Down
42 changes: 38 additions & 4 deletions src/StatusCodeInterface.php → constants.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,38 @@
<?php

namespace Fig\Http\Message;
/**
* Defines constants for common HTTP request methods.
*
* Usage:
*
* <code>
* use Fig\Http\Message\Request;
*
* class RequestFactory
* {
* public static function factory(
* $uri = '/',
* $method = Request\METHOD_GET,
* $data = []
* ) {
* }
* }
* </code>
*/
namespace Fig\Http\Message\Request
Copy link

@shadowhand shadowhand Oct 30, 2018

Choose a reason for hiding this comment

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

This namespace is kind of weird. Would be nice to write code like:

use const Http\Request\Method\GET;
use const Http\Response\Status\NOT_FOUND;

Though I suppose some of these constants might be reserved words in PHP <7.

Copy link
Author

@stefanotorresi stefanotorresi Dec 12, 2018

Choose a reason for hiding this comment

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

what about Fig\Http as a root instead of just Http? We need a vendor prefix.

Copy link

@shadowhand shadowhand Dec 12, 2018

Choose a reason for hiding this comment

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

Why is a vendor prefix necessary? @http-interop doesn't use one.

Copy link
Member

@Jean85 Jean85 Dec 13, 2018

Choose a reason for hiding this comment

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

I need to remind you the bylaw about this: https://www.php-fig.org/bylaws/psr-naming-conventions/

Special lighweight utility packages MAY be produced alongside PSRs and interfaces and be maintained and updated after the PSR has been accepted. These MUST be under the vendor namespace Fig.

Copy link
Author

@stefanotorresi stefanotorresi Dec 17, 2018

Choose a reason for hiding this comment

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

Why is a vendor prefix necessary?

oh come on Woody, really? Even if we didn't have the bylaw, it is not a great idea to claim a namespace as generic as Http... 😜

Copy link

@shadowhand shadowhand Dec 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

"Claim" a namespace? One package using a namespace does not prevent another package from using it. 😉

Copy link
Member

@michaelcullum michaelcullum Dec 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The whole point of namespacing and including vendors is exactly that, to avoid multiple packages/projects/vendors clashing unintentionally.

On packagist, for example, exactly this happens I believe (one package using a vendor name prevents others from using the same one).

Copy link
Author

@stefanotorresi stefanotorresi Dec 17, 2018

Choose a reason for hiding this comment

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

"Claim" a namespace? One package using a namespace does not prevent another package from using it.

Ok, "claim" is not entirely accurate, but that's what happens de-facto. Besides, it's asking for trouble!

{
const METHOD_HEAD = 'HEAD';
const METHOD_GET = 'GET';
const METHOD_POST = 'POST';
const METHOD_PUT = 'PUT';
const METHOD_PATCH = 'PATCH';
const METHOD_DELETE = 'DELETE';
const METHOD_PURGE = 'PURGE';
const METHOD_OPTIONS = 'OPTIONS';
const METHOD_TRACE = 'TRACE';
const METHOD_CONNECT = 'CONNECT';
}


/**
* Defines constants for common HTTP status code.
Expand All @@ -26,15 +58,17 @@
* Usage:
*
* <code>
* class ResponseFactory implements StatusCodeInterface
* use Fig\Http\Message\Response;
*
* class ResponseFactory
* {
* public function createResponse($code = self::STATUS_OK)
* public function createResponse($code = Response\STATUS_OK)
* {
* }
* }
* </code>
*/
interface StatusCodeInterface
namespace Response
{
// Informational 1xx
const STATUS_CONTINUE = 100;
Expand Down
34 changes: 0 additions & 34 deletions src/RequestMethodInterface.php
View file
Open in desktop

This file was deleted.