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

India states update #7949

Merged
merged 1 commit into from Jun 21, 2017

Conversation

Projects
None yet
6 participants
@ish6614
Contributor

ish6614 commented Jun 1, 2017

Verified from https://en.wikipedia.org/wiki/States_and_union_territories_of_India

Questions Answers
Branch? develop
Description? Added new states to India
Type? improvement
Category? LO
BC breaks? Does it break backward compatibility? no
Deprecations? Does it deprecate an existing feature? no
Fixed ticket? (optional) If this PR fixes a Forge ticket, please add its complete Forge URL.
How to test? Verify from https://en.wikipedia.org/wiki/States_and_union_territories_of_India

Important guidelines

@prestonBot

This comment has been minimized.

Show comment
Hide comment
@prestonBot

prestonBot Jun 1, 2017

Collaborator

Hello ish6614!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

Collaborator

prestonBot commented Jun 1, 2017

Hello ish6614!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@aleeks

This comment has been minimized.

Show comment
Hide comment
@aleeks

aleeks Jun 1, 2017

Contributor

ping @AlexEven

Contributor

aleeks commented Jun 1, 2017

ping @AlexEven

@maximebiloe

This comment has been minimized.

Show comment
Hide comment
@maximebiloe

maximebiloe Jun 1, 2017

Contributor

Hello @ish6614,

Can you check your indentation, please?
Also, can you rename your commit (not the PR) to respect our standards?

Thank you

Contributor

maximebiloe commented Jun 1, 2017

Hello @ish6614,

Can you check your indentation, please?
Also, can you rename your commit (not the PR) to respect our standards?

Thank you

@xBorderie

This comment has been minimized.

Show comment
Hide comment
@xBorderie

xBorderie Jun 7, 2017

Contributor

Thank you for your contribution!

Sadly, you now have malformed commits instead of just one commit:

  • the initial commit, which does not follow the PS naming convention,
  • one commit which does follow the naming convention (but should add a space after "LO:"),
  • three "merge" commits -- which if merged can break the repository's history!

Could you try and squash those commits into just one? Another explanation of squashing.

Thank you!

Contributor

xBorderie commented Jun 7, 2017

Thank you for your contribution!

Sadly, you now have malformed commits instead of just one commit:

  • the initial commit, which does not follow the PS naming convention,
  • one commit which does follow the naming convention (but should add a space after "LO:"),
  • three "merge" commits -- which if merged can break the repository's history!

Could you try and squash those commits into just one? Another explanation of squashing.

Thank you!

@ish6614

This comment has been minimized.

Show comment
Hide comment
@ish6614

ish6614 Jun 8, 2017

Contributor

@maximebiloe Could you please check the travis log. It says that it failed to fetch selenium server.
You might need to re run the CI test.

Contributor

ish6614 commented Jun 8, 2017

@maximebiloe Could you please check the travis log. It says that it failed to fetch selenium server.
You might need to re run the CI test.

@aleeks

This comment has been minimized.

Show comment
Hide comment
@aleeks

aleeks Jun 8, 2017

Contributor

Hello @ish6614
The build is ok, but your PR not.
Please, you need to do 1 unique commit for your change.

Contributor

aleeks commented Jun 8, 2017

Hello @ish6614
The build is ok, but your PR not.
Please, you need to do 1 unique commit for your change.

@xBorderie

This comment has been minimized.

Show comment
Hide comment
@xBorderie

xBorderie Jun 8, 2017

Contributor

Please follow my squashing advice above.

Contributor

xBorderie commented Jun 8, 2017

Please follow my squashing advice above.

@aleeks

This comment has been minimized.

Show comment
Hide comment
@aleeks

aleeks Jun 8, 2017

Contributor

Done.
@ish6614 Why do you unversed language iso_code ? en then ta now ?

Contributor

aleeks commented Jun 8, 2017

Done.
@ish6614 Why do you unversed language iso_code ? en then ta now ?

@vincentbz vincentbz requested a review from AlexEven Jun 19, 2017

@aleeks

This comment has been minimized.

Show comment
Hide comment
@aleeks

aleeks Jun 20, 2017

Contributor

Done

Contributor

aleeks commented Jun 20, 2017

Done

@xBorderie xBorderie added this to the 1.7.2.0 milestone Jun 20, 2017

@maximebiloe maximebiloe changed the title from Updated new states in India to India states update Jun 21, 2017

@maximebiloe maximebiloe merged commit 8b47986 into PrestaShop:develop Jun 21, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@maximebiloe

This comment has been minimized.

Show comment
Hide comment
@maximebiloe

maximebiloe Jun 21, 2017

Contributor

Thank you @ish6614

Contributor

maximebiloe commented Jun 21, 2017

Thank you @ish6614

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment