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

MAG2x: Changing long array syntax to the short syntax one. #153

Merged
merged 1 commit into from
Sep 18, 2018

Conversation

guzzilar
Copy link
Contributor

@guzzilar guzzilar commented Sep 18, 2018

⚠️ Note, this pull request requires PR #123 to be merged first.

1. Objective

screen shot 2561-09-18 at 14 54 14

There are code standard provided by Magento that all extensions have to follow in order to submit the code to their Magento Marketplace.

This pull request is aiming to update one of the standard (array syntax) to a proper one that Magento requires.

2. Description of change

  • Update all long-syntax-array to the short one.

3. Quality assurance

  1. Install https://github.com/magento/marketplace-eqp.
  2. Execute the command vendor/bin/phpcs /path/to/your/extension --standard=MEQP2.
  3. Search for a warning message Short array syntax must be used to define arrays.

Before change, we should see these ones
screen shot 2561-09-18 at 14 54 14

After change, we should see none of them
screen shot 2561-09-18 at 15 01 40

4. Impact of the change

Nothing in a user perspective, but it will make our extension be able to pass the Magento Code Quality Standard and able to submit to the Magento Marketplace.

5. Priority of change

Normal

6. Additional Notes

];
}

return [ self::CARD => $method->getAdditionalInformation(CreditCardDataObserver::TOKEN) ];
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably clearer to use if ... else, or better still a single return with a conditional operator

@@ -98,7 +98,7 @@ public function reload()
class MockOmiseObject implements \ArrayAccess
{
// Store the attributes of the object.
protected $_values = array();
protected $_values = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Protected var name starting with _

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonrandy Thanks
Code-refactoring PR will be coming in soon!

Copy link
Contributor

@jacstn jacstn Sep 18, 2018

Choose a reason for hiding this comment

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

@guzzilar no necessary to change in pr only few places.. I propose to change all in entire plugin..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Jacekstan yup, so changing entire plugin will be coming in an another pull request once tesco-lotus feature is merged (or maybe before) (I updated array syntax & opened this pull request just to continue from a conversation at the previous PR #123)

According to Magento Extension Quality Program Coding Standard (https://github.com/magento/marketplace-eqp).
All long-array-syntax are encouraged to be updated to the short syntax one.
@guzzilar guzzilar merged commit 67ab7d1 into master Sep 18, 2018
@guzzilar guzzilar deleted the update-array-syntax branch September 18, 2018 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants