-
Notifications
You must be signed in to change notification settings - Fork 4
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
Make it a composer project so it can, you know, work... #1
Conversation
@@ -0,0 +1,2 @@ | |||
/vendor/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vendor*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's how the composer init
command sets it, but ¯\_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then should the idea/
directory be the same: .idea*
?
I think I prefer how it is now, you're trying to exclude the directory, you don't need a wildcard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/ErrorCatalog.php
Outdated
* @param $namespace | ||
* @param string $language | ||
*/ | ||
public function __construct($namespace, $language = 'en-US') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$this->language = $language
src/ErrorCatalog.php
Outdated
* @return ErrorCatalogItem | ||
*/ | ||
public function getItem($name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should this method return if it doesn't find the item?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now, return null;
. Maybe we should add a default at a later stage
src/ErrorSpecIssue.php
Outdated
/** | ||
* @var string The JSON pointer to identify the issue | ||
*/ | ||
protected $jsonPointer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not really a json pointer though is it? What exactly is this extra identifier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe call this reference
@@ -0,0 +1,2 @@ | |||
/vendor/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/ErrorSpecIssue.php
Outdated
/** | ||
* @var string The JSON pointer to identify the issue | ||
*/ | ||
protected $jsonPointer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe call this reference
src/ErrorCatalog.php
Outdated
* @return ErrorCatalogItem | ||
*/ | ||
public function getItem($name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now, return null;
. Maybe we should add a default at a later stage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.