8000 CS: use nowdoc instead of heredoc by gharlan · Pull Request #17086 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

CS: use nowdoc instead of heredoc #17086

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

Merged
merged 1 commit into from
Dec 28, 2015
Merged

Conversation

gharlan
Copy link
Contributor
@gharlan gharlan commented Dec 21, 2015
Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

if this is accepted, we could add the fixer to symfony level of php-cs-fixer: PHP-CS-Fixer/PHP-CS-Fixer#1580

@@ -1,7 +1,7 @@
This template is used for translation message extraction tests< 8000 /span>
<?php echo $view['translator']->trans('single-quoted key') ?>
<?php echo $view['translator']->trans('double-quoted key') ?>
<?php echo $view['translator']->trans(<<<EOF
<?php echo $view['translator']->trans(<<<'EOF'
Copy link
Member

Choose a reason for hiding this comment

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

this must be reverted. It is precisely testing the extraction from heredoc (extracting from nowdoc is tested 3 lines later)

@gharlan gharlan force-pushed the nowdoc branch 2 times, most recently from f5f8fa0 to 7ce6b7d Compare December 21, 2015 11:14
@gharlan gharlan changed the title use nowdoc instead of heredoc CS: use nowdoc instead of heredoc Dec 21, 2015
@nicolas-grekas
Copy link
Member

Personally, I'm -0 on this. I see no value here in comparison to the merge conflicts and added pressure on authors...

@gharlan
Copy link
Contributor Author
gharlan commented Dec 22, 2015

We already force single quotes instead of double quotes (when possible). So in my opinion it would be consistent to do it the same way with nowdoc vs. heredoc.


return \$this->doGenerate(\$variables, \$defaults, \$requirements, \$tokens, \$parameters, \$name, \$referenceType, \$hostTokens);
return $this->doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens);
Copy link
Member

Choose a reason for hiding this comment

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

so cleaner with nowdoc ;)

@nicolas-grekas
Copy link
Member

👍 (not a big one but still :) )

@stof
Copy link
Member
stof commented Dec 28, 2015

👍

@nicolas-grekas
Copy link
Member

Thank you @gharlan.

@nicolas-grekas nicolas-grekas merged commit 3dca549 into symfony:2.3 Dec 28, 2015
nicolas-grekas added a commit that referenced this pull request Dec 28, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

CS: use nowdoc instead of heredoc

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | —
| License       | MIT
| Doc PR        | —

if this is accepted, we could add the fixer to symfony level of php-cs-fixer: PHP-CS-Fixer/PHP-CS-Fixer#1580

Commits
-------

3dca549 use nowdoc instead of heredoc
@nicolas-grekas
Copy link
Member

I merged 2.3 up to master without looking for more heredoc to change, which means they may be more in 2.7 and up.

@keradus
Copy link
Member
keradus commented Dec 28, 2015

Great to hear @nicolas-grekas
That is how CS fixes usually works here ;) fabbot.io will catch more changes in PRs if needed.

@gharlan gharlan deleted the nowdoc branch December 30, 2015 20:00
ostrolucky pushed a commit to ostrolucky/symfony that referenced this pull request Mar 25, 2018
This PR was merged into the 2.3 branch.

Discussion
----------

CS: use nowdoc instead of heredoc

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | —
| License       | MIT
| Doc PR        | —

if this is accepted, we could add the fixer to symfony level of php-cs-fixer: PHP-CS-Fixer/PHP-CS-Fixer#1580

Commits
-------

3dca549 use nowdoc instead of heredoc
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.

5 participants
0