Skip to content

Conversation

@amirkhiz
Copy link

  • Add methods PHPDocs
  • Add ext-ctype package in composer.json

/build
/vendor

/.idea/
Copy link

Choose a reason for hiding this comment

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

Needs a new line following this

"require": {
"php": ">=5.3.0",
"lib-pcre": ">=7.3"
"php": ">=5.6.0",
Copy link

Choose a reason for hiding this comment

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

This is a potentially breaking change, should we do this?

+ Update namespace
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

This appears to be formatting and renaming namespace/packages only (to the wrong name). Suggest rejecting PR, however, this repo seems to be unmaintained so not an issue.

```json
"require": {
"inacho/php-credit-card-validator": "1.*"
"habil/php-credit-card-validator": "1.*"
Copy link

Choose a reason for hiding this comment

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

This should probably stay inacho since it's referring to this repo.

@@ -1,5 +1,5 @@
{
"name": "inacho/php-credit-card-validator",
"name": "habil/php-credit-card-validator",
Copy link

Choose a reason for hiding this comment

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

Should stay "inacho".

"autoload": {
"psr-4": {
"Inacho\\": "src/"
"Sivax\\": "src/"
Copy link

Choose a reason for hiding this comment

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

Should stay "Inacho".

*/

namespace Inacho;
namespace Sivax;
Copy link

Choose a reason for hiding this comment

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

Wrong namespace

/**
* Class CreditCard
* @package Inacho
* @package Sivax
Copy link

Choose a reason for hiding this comment

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

Wrong package name

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.

4 participants