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

#11328 : app:config:dump adds extra space every time in multiline array value #11452

Conversation

adrian-martinez-interactiv4
Copy link
Contributor

@adrian-martinez-interactiv4 adrian-martinez-interactiv4 commented Oct 14, 2017

Related PRs:

app:config:dump adds extra space every time in multiline array value

Description

Rewrite PhpFormatter::format method, to avoid add extra spaces

Fixed Issues (if relevant)

  1. app:config:dump adds extra space every time in multiline array value #11328: app:config:dump adds extra space every time in multiline array value

Manual testing scenarios

Described in issue #11328

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)

@davidalger davidalger self-assigned this Oct 18, 2017
@davidalger davidalger added this to the October 2017 milestone Oct 18, 2017
@davidalger davidalger added develop Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Oct 18, 2017
@davidalger
Copy link
Member

@adrian-martinez-interactiv4 I see two additional commits on #11451 (2.1-develop version of this). Can you replay those onto this PR as well?

@adrian-martinez-interactiv4
Copy link
Contributor Author

@davidalger I'll check this

@magento-engcom-team magento-engcom-team changed the base branch from develop to 2.3-develop October 20, 2017 15:32
@adrian-martinez-interactiv4
Copy link
Contributor Author

@davidalger Some changes only applied to 2.1 version, that's why it has more commits than 2.2 and develop PRs. Anyway, I have checked changed files for differences, and I have unified the code between all versions in last added commit. This should be ready for review.

okorshenko pushed a commit that referenced this pull request Oct 24, 2017
@okorshenko okorshenko self-assigned this Oct 24, 2017
@okorshenko okorshenko added bug report Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release labels Oct 24, 2017
@adrian-martinez-interactiv4
Copy link
Contributor Author

adrian-martinez-interactiv4 commented Oct 24, 2017

@okorshenko I can see you referenced this PR in 09b719b, and changes are merged and applied in 2.3-develop branch in c6e891d, in @magento-engcom-team internal PR 1607. Just for knowing, why doesn't this PR show as Merged and Closed? Maybe when several commits are merged into base branch via internal PR instead of merging MAGETWO-* commits, the PR does not know the change has been applied?

It's just for my information, to know if I should close this PR, and to highlight this way PR maintenance is harder if they don't get closed automatically.

Thank you in advance.

@okorshenko
Copy link
Contributor

Hi @adrian-martinez-interactiv4 I'm working on this right now. Will be closed/merged soon. Don't worry

@okorshenko okorshenko merged commit 1740f62 into magento:2.3-develop Oct 25, 2017
okorshenko pushed a commit that referenced this pull request Oct 25, 2017
@adrian-martinez-interactiv4 adrian-martinez-interactiv4 deleted the FR#11328-APP-CONFIG-DUMP-EXTRA-SPACES branch October 27, 2017 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Award: test coverage bug report Progress: accept Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants