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

Allow Upper Case property names#22265

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

Closed
insekticid wants to merge1 commit intosymfony:masterfrominsekticid:patch-1
Closed

Allow Upper Case property names#22265

insekticid wants to merge1 commit intosymfony:masterfrominsekticid:patch-1

Conversation

@insekticid
Copy link
Contributor

@insekticidinsekticid commentedApr 4, 2017•
edited
Loading

ReflectionExtractor::getProperties() returns $id instead of $Id. It is bad naming convention, but is possible

class Entity {    protected $Id;    public function getId()    {        return $this->Id;    }}
QA
Branch?2.8
Bug fix?yes
New feature?no
BC breaks?yes
Deprecations?no
Tests pass?no
LicenseMIT

api-platform/core#1026

insekticid added a commit to insekticid/core that referenced this pull requestApr 4, 2017
ReflectionExtractor::getProperties() returns $id instead of $Id. It is bad naming convention, but is possible```class Entity {    protected $Id;    public function getId()    {        return $this->Id;    }}```symfony/symfony#22265
Copy link
Member

@dunglasdunglas left a comment

Choose a reason for hiding this comment

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

👍

@dunglas
Copy link
Member

Can you cover this with an unit test?

@insekticid
Copy link
ContributorAuthor

insekticid commentedApr 4, 2017•
edited
Loading

Added phpunit tests
I have fixed code style and updated code with master + force push to my patch-1 branch

You need manually rerun build checks:

If you have write access to the repo: On the build's detail screen, there is a button ↻ with the tooltip "Restart Build".

@nicolas-grekas
Copy link
Member

small rebase needed

@nicolas-grekasnicolas-grekas modified the milestones:3.3,2.7Apr 4, 2017
@nicolas-grekas
Copy link
Member

should be merged on 2.7, isn't it?

@nicolas-grekas
Copy link
Member

👍

@insekticid
Copy link
ContributorAuthor

Symfony 2.7+ So what to do now? It will be automatically merged into Symfony 3 too?

@fabpot
Copy link
Member

@insekticid That's correct. We are merging bug fixes in the lowest branch where the bug exist and then merge it back to all other branches (2.8, 3.2, and master right now). Can you change the target of the PR and rebase it? Thanks.

ReflectionExtractor::getProperties() returns $id instead of $Id. It is bad naming convention, but is possible```class Entity {    protected $Id;    public function getId()    {        return $this->Id;    }}```# Conflicts:#src/Symfony/Component/PropertyInfo/Tests/Extractors/ReflectionExtractorTest.php#src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php
@insekticid
Copy link
ContributorAuthor

@fabpot PropertyInfo exists since Sf 2.8 so I remade patch-1 for remotes/origin/2.8

@fabpot
Copy link
Member

@insekticid You forgot to change the branch of the PR

@insekticid
Copy link
ContributorAuthor

@fabpot where? in commit message?

@fabpot
Copy link
Member

Thank you@insekticid.

fabpot added a commit that referenced this pull requestApr 4, 2017
This PR was submitted for the master branch but it was merged into the 2.8 branch instead (closes#22265).Discussion----------Allow Upper Case property namesReflectionExtractor::getProperties() returns $id instead of $Id. It is bad naming convention, but is possible```class Entity {    protected $Id;    public function getId()    {        return $this->Id;    }}```| Q             | A| ------------- | ---| Branch?       | 2.8| Bug fix?      | yes| New feature?  | no| BC breaks?    | yes| Deprecations? | no| Tests pass?   | no| License       | MITapi-platform/core#1026Commits-------2fe1631 Allow Upper Case property names
@fabpot
Copy link
Member

Nevermind, I've done the switch when merging. FYI, that's just after the PR title, the branch you asked this PR to be merged in.

@fabpotfabpot closed thisApr 4, 2017
@insekticid
Copy link
ContributorAuthor

insekticid commentedApr 4, 2017•
edited
Loading

@fabpot aha, so next time a I should close old (this) pull req. and made new one to 2.8 branch?

edit: found it :) Simply Edit this pull

@fabpot
Copy link
Member

No need to close it. You can now (that was changed recentyl on Github) update the target on a PR.

insekticid reacted with thumbs up emoji

This was referencedApr 5, 2017
fabpot added a commit that referenced this pull requestApr 29, 2017
…ticid)This PR was merged into the 2.8 branch.Discussion----------Allow Upper Case property names in ObjectNormalizer| Q             | A| ------------- | ---| Branch?       | 2.8| Bug fix?      | yes| New feature?  | no| BC breaks?    | yes| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#22547| License       | MITSame problem that has been fixed here#22265and hereapi-platform/core#1037ObjectNormalizer returns $id instead of $Id. It is bad naming convention, but is possible```phpclass Entity {    protected $Id;    public function getId()    {        return $this->Id;    }}```Commits-------b2b4faa Allow Upper Case property names in ObjectNormalizer
symfony-splitter pushed a commit to symfony/serializer that referenced this pull requestApr 29, 2017
| Q                | A| ---------------- | -----| Bug report?      | yes| Feature request? | no| BC Break report? | yes| RFC?             | no| Symfony version  | 2.8.19Same problem that has been fixed heresymfony/symfony#22265and hereapi-platform/core#1037ObjectNormalizer returns $id instead of $Id. It is bad naming convention, but is possible```phpclass Entity {    protected $Id;    public function getId()    {        return $this->Id;    }}```
symfony-splitter pushed a commit to symfony/serializer that referenced this pull requestApr 29, 2017
…ticid)This PR was merged into the 2.8 branch.Discussion----------Allow Upper Case property names in ObjectNormalizer| Q             | A| ------------- | ---| Branch?       | 2.8| Bug fix?      | yes| New feature?  | no| BC breaks?    | yes| Deprecations? | no| Tests pass?   | yes| Fixed tickets | #22547| License       | MITSame problem that has been fixed heresymfony/symfony#22265and hereapi-platform/core#1037ObjectNormalizer returns $id instead of $Id. It is bad naming convention, but is possible```phpclass Entity {    protected $Id;    public function getId()    {        return $this->Id;    }}```Commits-------b2b4faa3c0 Allow Upper Case property names in ObjectNormalizer
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@dunglasdunglasdunglas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

2.7

Development

Successfully merging this pull request may close these issues.

6 participants

@insekticid@dunglas@nicolas-grekas@fabpot@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp