Skip to content

[Yaml] Fix improper comments removal #15860

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
Oct 1, 2015
Merged

[Yaml] Fix improper comments removal #15860

merged 1 commit into from
Oct 1, 2015

Conversation

ogizanagi
Copy link
Contributor

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

This tries to fix #15857 .

Honestly, I don't have any idea of the regressions it might introduce. Tests are passing, so if this code had any reason to exist, tests covering it are certainly missing :/

Any hint ?

@skigun
Copy link
Contributor

skigun commented Sep 21, 2015

👍

@fabpot
Copy link
Member

fabpot commented Oct 1, 2015

Thank you @ogizanagi.

@fabpot fabpot merged commit 0e24fc5 into symfony:2.3 Oct 1, 2015
fabpot added a commit that referenced this pull request Oct 1, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

[Yaml] Fix improper comments removal

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

This tries to fix #15857 .

Honestly, I don't have any idea of the regressions it might introduce. Tests are passing, so if this code had any reason to exist, tests covering it are certainly missing :/

Any hint ?

Commits
-------

0e24fc5 [Yaml] Fix improper comments removal inside strings
@ogizanagi ogizanagi deleted the yaml/fix_improper_comments_removal branch October 1, 2015 21:12
fabpot added a commit that referenced this pull request Oct 2, 2015
@fabpot
Copy link
Member

fabpot commented Oct 2, 2015

reverted as it introduces regressions

@ogizanagi ogizanagi restored the yaml/fix_improper_comments_removal branch October 2, 2015 11:13
fabpot added a commit that referenced this pull request Oct 2, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

[Yaml] Add regression test for comments indents

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

This is related to #16065, #15857 and #15860  (last one has been reverted due to this regression)

Commits
-------

7b9d519 [Yaml] Add regression test for comments indents
fabpot added a commit that referenced this pull request Oct 2, 2015
* 2.3:
  [Yaml] Add regression test for comments indents
  Revert "bug #15860 [Yaml] Fix improper comments removal (ogizanagi)"
fabpot added a commit that referenced this pull request Oct 2, 2015
* 2.7:
  [Yaml] Add regression test for comments indents
  Revert "bug #15860 [Yaml] Fix improper comments removal (ogizanagi)"
fabpot added a commit that referenced this pull request Oct 2, 2015
* 2.8:
  [Yaml] Add regression test for comments indents
  Fix the DomCrawler tests
  Revert "bug #15860 [Yaml] Fix improper comments removal (ogizanagi)"
This was referenced Oct 27, 2015
@ogizanagi ogizanagi deleted the yaml/fix_improper_comments_removal branch October 27, 2015 20:41
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.

4 participants