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

Issue #7988 Typo changed also added comments for each index, getters and setters. #9484

Closed
wants to merge 6 commits into from

Conversation

arshadpkm
Copy link
Contributor

@arshadpkm arshadpkm commented May 3, 2017

Description

Issue #7988 Typo changed also added comments for each index, getters and setters.

Fixed Issues (if relevant)

  1. Typo in variable name: billingAddressFromData vs. billingAddressFormData #7988: Typo in variable name: billingAddressFromData vs. billingAddressFormData

Manual testing scenarios

  1. Not applicable

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented May 3, 2017

CLA assistant check
All committers have signed the CLA.

@adragus-inviqa
Copy link
Contributor

I'm not sure comments add much value here. Comments usually don't. I'd rather you take them out.

What happens if the logic changes and the values are not taken from local storage? You'd have to update the comments, too. Suddently, you have two things to maintain.

@okorshenko okorshenko changed the title #7988 Pull Request Issue #7988 Typo changed also added comments for each index, getters and setters. May 3, 2017
'selectedBillingAddress': null,
'billingAddressFormData': null,
'newCustomerBillingAddress': null
'selectedShippingAddress': null, // Selected shipping address pullled from local storage (Persistence)
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. remove space between // and Selected (fix all cases)
  2. replace local storage (Persistence) and local storage --> persistence storage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in my new commit

};
saveData(checkoutData);
}

return {
/**
* Setting the selected shipping address pulled from local storage
Copy link
Contributor

Choose a reason for hiding this comment

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

add . at the end of the comment. (all cases)

@okorshenko okorshenko self-assigned this May 3, 2017
@okorshenko okorshenko added this to the May 2017 milestone May 3, 2017
@adragus-inviqa
Copy link
Contributor

If you're going to do it, at least fix the pullled typo.

@ishakhsuvarov
Copy link
Contributor

Hi @arshadpkm
Please sync your branch with the latest develop and fix merge conflicts
Thanks!

@arshadpkm
Copy link
Contributor Author

@ishakhsuvarov Fixed merge conflicts

magento-team pushed a commit that referenced this pull request Jun 13, 2017
magento-team pushed a commit that referenced this pull request Jun 13, 2017
…index, getters and setters. #9484

 - fixed code style issue
magento-team pushed a commit that referenced this pull request Jun 13, 2017
…index, getters and setters. #9484

 - fixed code style issue
magento-team pushed a commit that referenced this pull request Jun 13, 2017
…index, getters and setters. #9484

 - fixed merge conflict
magento-team pushed a commit that referenced this pull request Jun 13, 2017
@okorshenko
Copy link
Contributor

Hi @arshadpkm thank you for your contribution. Your Pull Request has been merged.

@okorshenko okorshenko closed this Jun 13, 2017
AntonEvers pushed a commit to AntonEvers/magento2 that referenced this pull request Jun 14, 2017
…-9924

* upstream/develop: (60 commits)
  Fix typo in comment
  Move prefix and suffix default values to a new PR
  MAGETWO-68877: Issue magento#7988 Typo changed also added comments for each index, getters and setters. magento#9484
  Revert "MAGETWO-69728: Fixes layered navigation options being cached using the wrong store id. magento#9873"
  MAGETWO-67500: setup:di:compile returns exit code 0 if errors are found magento#7780
  Fix prefix, middle name and suffix were not prefilled in the checkout
  add middle name to checkout address html templates magento#8878
  Using Command output as message which actually provides more information for debugging than just the path
  Handling CLI error as a failure when validating composer.json file
  MAGETWO-69805: Return array of blocks as items instead of array of arrays magento#9157
  MAGETWO-69666: Return array of pages as items instead of array of arrays magento#9823
  MAGETWO-69723: Email to a Friend feature magento#9824
  MAGETWO-69539: PHP "soap" extension is not declared in composer.json but can be used by Magento modules
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-63054: [Catalog] MSRP field is not displayed for bundle products with fixed price
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  ...
AntonEvers pushed a commit to AntonEvers/magento2 that referenced this pull request Jun 14, 2017
* develop: (60 commits)
  Fix typo in comment
  Move prefix and suffix default values to a new PR
  MAGETWO-68877: Issue magento#7988 Typo changed also added comments for each index, getters and setters. magento#9484
  Revert "MAGETWO-69728: Fixes layered navigation options being cached using the wrong store id. magento#9873"
  MAGETWO-67500: setup:di:compile returns exit code 0 if errors are found magento#7780
  Fix prefix, middle name and suffix were not prefilled in the checkout
  add middle name to checkout address html templates magento#8878
  Using Command output as message which actually provides more information for debugging than just the path
  Handling CLI error as a failure when validating composer.json file
  MAGETWO-69805: Return array of blocks as items instead of array of arrays magento#9157
  MAGETWO-69666: Return array of pages as items instead of array of arrays magento#9823
  MAGETWO-69723: Email to a Friend feature magento#9824
  MAGETWO-69539: PHP "soap" extension is not declared in composer.json but can be used by Magento modules
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-63054: [Catalog] MSRP field is not displayed for bundle products with fixed price
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  MAGETWO-67087: Saving category deletes url-rewrites for products in anchor categories
  ...
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.

6 participants