Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Yaml] Implement $blockChompingIndicator for TaggedValue multi-line literal blocks#40431

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
natepage wants to merge5 commits intosymfony:7.4
base:7.4
Choose a base branch
Loading
fromnatepage:bugfix/yaml-support-block-chomping-indicator-for-tagged-value

Conversation

natepage
Copy link
Contributor

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
Tickets-
LicenseMIT

4c513c2 introduced the notion of$blockChompingIndicator for multi-line literal blocks.

This PR implements the same logic for TaggedValue multi-line literal blocks.

@carsonbotcarsonbot added this to the4.4 milestoneMar 10, 2021
@carsonbotcarsonbot changed the title[YAML] Implement $blockChompingIndicator for TaggedValue multi-line literal blocksImplement $blockChompingIndicator for TaggedValue multi-line literal blocksMar 10, 2021
@carsonbotcarsonbot changed the titleImplement $blockChompingIndicator for TaggedValue multi-line literal blocks[Yaml] Implement $blockChompingIndicator for TaggedValue multi-line literal blocksMar 10, 2021
@natepage
Copy link
ContributorAuthor

@xabbuh Hi, not sure whyTests/Integration (7.4) is failing, any idea what I could have done wrong?

@nicolas-grekas
Copy link
Member

Can you please add a few more test cases to cover all the branches?

@natepage
Copy link
ContributorAuthor

natepage commentedMar 17, 2021•
edited
Loading

@nicolas-grekas Thank you for your reply, sorry I just want to clarify.. What do you mean by "cover all the branches"?

  • Are you talking about repo branches for different versions of symfony? (e.g. 4.4, 5.0, 5.1, 5.2)
  • Or, are you talking about "code branches/paths" and improve the coverage?

Thank you

@nicolas-grekas
Copy link
Member

Or, are you talking about "code branches/paths" and improve the coverage?

that yes :)

@xabbuh
Copy link
Member

@natepage You can ignore that build failure. It's not related to your changes.

@xabbuh
Copy link
Member

@natepage Do you need help with writing a test? :)

@natepage
Copy link
ContributorAuthor

@xabbuh I should be ok for the test, any tips on raising a kid? 😄

Sorry I had a few concurrent priorities just after creating the PR.

Having a look at the code, I've realised there was a difference between scalar and tagged values regarding trailing newlines so I've updated the code to match, and reused your testtestDumpTrailingNewlineInMultiLineLiteralBlocks() but with TaggedValues and make sure the result is consistent.

@nicolas-grekas The Dumper is now 100% covered.

xabbuh and chalasr reacted with laugh emoji

@natepagenatepage requested a review fromxabbuhApril 22, 2021 12:45
@natepagenatepage requested a review fromstofMay 3, 2021 14:02
@OskarStark
Copy link
Contributor

@xabbuh@stof open for a final review? CI is green

@fabpot
Copy link
Member

As this is a new feature, this should target the main branch (5.4 as of today).

@fabpotfabpot modified the milestones:4.4,5.4Aug 26, 2021
@natepagenatepage changed the base branch from4.4 to5.4August 26, 2021 21:56
@natepage
Copy link
ContributorAuthor

@fabpot Thank you for your feedback, I've changed the base branch to 5.4

@fabpotfabpot removed this from the5.4 milestoneNov 16, 2021
@fabpotfabpot added this to the6.1 milestoneNov 16, 2021
@fabpotfabpot modified the milestones:6.1,6.2May 20, 2022
@nicolas-grekasnicolas-grekas modified the milestones:6.2,6.3Nov 5, 2022
@fabpot
Copy link
Member

@xabbuh Can you have a look at this one please?

@nicolas-grekasnicolas-grekas modified the milestones:6.3,6.4May 23, 2023
@nicolas-grekasnicolas-grekas modified the milestones:6.4,7.1Nov 15, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@maxheliasmaxheliasmaxhelias approved these changes

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

@stofstofAwaiting requested review from stof

Assignees
No one assigned
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.

8 participants
@natepage@nicolas-grekas@xabbuh@OskarStark@fabpot@stof@maxhelias@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp