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

[Form] Php doc fixes and cs + optimizations#30879

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

Merged
fabpot merged 1 commit intosymfony:3.4fromHeahDude:fix/form-doc
Apr 6, 2019

Conversation

@HeahDude
Copy link
Contributor

QA
Branch?3.4
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets~
LicenseMIT
Doc PR~

Some micro optimizations may target master with some profiling but I would like some reviews first. Thanks!

Copy link
Contributor

@dmaicherdmaicher left a comment

Choose a reason for hiding this comment

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

👍

}

if (null ===$this->getName() ||'' ===$this->getName()) {
if (!$this->name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what if the name is'0'? 😄 Can it happen?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Yes it can, nice catch!

Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we add a testcase or is there already one?

Copy link
Contributor

@dmaicherdmaicherApr 6, 2019•
edited
Loading

Choose a reason for hiding this comment

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

Ok so I would just writeif (trim($this->name) === '')

As discussed let's avoid the function call 😄

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

No function call :)

@HeahDude
Copy link
ContributorAuthor

Thanks for the reviews :). Comments addressed, ready for another round!

Copy link
Contributor

@vudaltsovvudaltsov left a comment

Choose a reason for hiding this comment

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

Please, fixchained -> chain

@fabpot
Copy link
Member

Thank you@HeahDude.

@fabpotfabpot merged commit56429a6 intosymfony:3.4Apr 6, 2019
fabpot added a commit that referenced this pull requestApr 6, 2019
This PR was merged into the 3.4 branch.Discussion----------[Form] Php doc fixes and cs + optimizations| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | no| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks?    | no     <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets | ~| License       | MIT| Doc PR        | ~<!--Write a short README entry for your feature/bugfix here (replace this comment block.)This will help people understand your PR and can be used as a start of the Doc PR.Additionally: - Bug fixes must be submitted against the lowest branch where they apply   (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the master branch.-->Some micro optimizations may target master with some profiling but I would like some reviews first. Thanks!Commits-------56429a6 [Form] various minor fixes
@xabbuhxabbuh mentioned this pull requestApr 6, 2019
xabbuh added a commit that referenced this pull requestApr 6, 2019
This PR was merged into the 3.4 branch.Discussion----------[Form] fix tests| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#30879| License       | MIT| Doc PR        |Commits-------83c661d fix tests
This was referencedApr 16, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@OskarStarkOskarStarkOskarStark requested changes

@stofstofstof approved these changes

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

+5 more reviewers

@sstoksstoksstok left review comments

@dmaicherdmaicherdmaicher left review comments

@dbrumanndbrumanndbrumann left review comments

@vudaltsovvudaltsovvudaltsov approved these changes

@SimperfitSimperfitSimperfit approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

11 participants

@HeahDude@fabpot@stof@sstok@dmaicher@OskarStark@vudaltsov@dbrumann@Simperfit@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp