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

[PropertyInfo] Add Component Documentation#5974

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
weaverryan merged 5 commits intosymfony:2.8fromzanbaldwin:feature/component-property-info
Aug 20, 2016
Merged

[PropertyInfo] Add Component Documentation#5974

weaverryan merged 5 commits intosymfony:2.8fromzanbaldwin:feature/component-property-info
Aug 20, 2016

Conversation

@zanbaldwin
Copy link
Member

@zanbaldwinzanbaldwin commentedDec 7, 2015
edited
Loading

QA
Doc fix?no
New docs?yes (symfony/symfony#15858)
Applies to2.8,3.0
Fixed ticketsN/A

This is still awork in progress, but I'd like feedback from anyone regarding structure, grammar and any other improvements while I continue. After speaking with@dunglas, I'm creating a new PR due our branches differing by about 400 commits.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

As pointed out by a friend in the Philippines, the idiomblurs the lines is not easily understood by non-native English speakers. Need to consider an alternative.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about "works with both"?

dunglas added a commit to symfony/symfony that referenced this pull requestDec 14, 2015
…actor (zanderbaldwin)This PR was squashed before being merged into the 2.8 branch (closes#16911).Discussion----------[PropertyInfo] Update List Information from ReflectionExtractor| Q             | A| ------------- | ---| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#16889| License       | MIT| Doc PR        |symfony/symfony-docs#5974Unless a property with the same casing exists, lowercase the first letter of a property name extracted from a method. From what I understand, we don't actually need to support `snake_case` at all in the PropertyInfo component.I cannot think of any use-case where PropertyAccess would be used to get/set a property value *before* using PropertyInfo to find out information about the property and the values it supports.Since PropertyInfo supports the discovery of property names, which can then be used as identifiers in PropertyAccess, there should be no need to support a naming strategy to map property identifiers from one naming convention to another.---Running `$reflectionExtractor->getProperties($class)` with the following classes:```phpclass X{    public $a;    public $b;    public function getA() {}}// Result: array('a', 'b');``````phpclass Y{    public $A;    public $b;    public function setA() {}}// Result: array('A', 'b');``````phpclass Y{    public $username;    protected $emailAddress;    public $password;    public function getEmailAddress() {}    public function isActive() {}}// Result: array('username', 'emailAddress', 'password', 'active');```Commits-------b2da76c [PropertyInfo] Update List Information from ReflectionExtractor
Copy link
Member

Choose a reason for hiding this comment

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

It is also able to leverage adders and removers.

Copy link
Contributor

Choose a reason for hiding this comment

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

This list of example method names is not illustrative. I think some actual examples would be better.

@dunglas
Copy link
Member

Good job! I left some minor comments but 👍

@zanbaldwin
Copy link
MemberAuthor

Considering I failed English at school, the only language I can speak, thanks!
I'll try to finish this along with the edits within the next couple of days.

@zanbaldwin
Copy link
MemberAuthor

Back in Europe, finally have internet access, any comments on this so far? /cc@dunglas.

@zanbaldwinzanbaldwin changed the title(WIP) [PropertyInfo] Add Component Documentation[PropertyInfo] Add Component DocumentationJan 22, 2016
@TomasVotruba
Copy link

This looks good to me. Anything more that needs to be done?

@zanbaldwin
Copy link
MemberAuthor

TheCreating Your Own Extractors section could possibly be expanded but I could never figure out how to word it :P

@TomasVotruba
Copy link

I think that might be next, advanced article.

I'd add just this simple "How to use this" version. It's really missed at the moment.

The PropertyInfo component provides the functionality to get information about class properties using metadata of
popular sources.

Whilst the :doc:`PropertyAccess component</components/property_access/introduction>` allows you to read and write values
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I'll edit that now.

@wouterj
Copy link
Member

Hi @zanderbaldwin!

I'm sorry that it took so long before one of us reviewed this PR. I think it's a nice start. Can you please fix the comments I added to this PR and make sure lines are wrapped after the first word that crosses the 72th character (they are a bit too long atm)? Thanks already!

If you don't have time, just comment and we'll continue this PR.

status: needs work

@zanbaldwin
Copy link
MemberAuthor

Hi guys, I've made the changes that were suggested. Does anyone have any other input?

@dunglas
Copy link
Member

Looks good to me. You did a very good job documenting this component, thank you!

@dunglas
Copy link
Member

@zanderbaldwin can you rebase your PR?

@xabbuh@wouterj@weaverryan can this PR be merged? It would be nice to have the doc for this component online.

@zanbaldwin
Copy link
MemberAuthor

Rebased, though you might want to check that I've solved all the conflicts correctly...

The PropertyInfo component provides the functionality to get information
about class properties using metadata of popular sources.

While the:doc:`PropertyAccess component</components/property_access/introduction>`
Copy link
Member

Choose a reason for hiding this comment

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

The path should be updated to/components/property_info (it's why Travis fails)

Copy link
Contributor

@teohhanhuiteohhanhuiAug 16, 2016
edited
Loading

Choose a reason for hiding this comment

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

*/components/property_access

dunglas reacted with thumbs up emoji
@zanbaldwin
Copy link
MemberAuthor

Got the build working, would you like the 5 commits squashed into 1?

@dunglas
Copy link
Member

dunglas commentedAug 16, 2016
edited
Loading

Commits can be squashed by the merger.

Status: reviewed

@weaverryanweaverryan merged commita0409d7 intosymfony:2.8Aug 20, 2016
weaverryan added a commit that referenced this pull requestAug 20, 2016
This PR was merged into the 2.8 branch.Discussion----------[PropertyInfo] Add Component Documentation| Q             | A| ------------- | ---| Doc fix?      | no| New docs?     | yes (symfony/symfony#15858)| Applies to    | `2.8`, `3.0`| Fixed tickets | N/AThis is still a *work in progress*, but I'd like feedback from anyone regarding structure, grammar and any other improvements while I continue. After speaking with@dunglas, I'm creating a new PR due our branches differing by about 400 commits.Commits-------a0409d7 Update Reference to Service Container Tags78ea4a5 Update PropertyAccess Component Path7b786af Fix Incorrect Indent96b139b Suggested Changescf7a0ea Add PropertyInfo Component Documentation
@weaverryan
Copy link
Member

Merged! This is great! I was very much looking forward to the docs for this, but bootstrapping the docs for a new component is a lot of work :). So, thank you!

I opened#6896 with some proofreading tweaks that I'm making. I'll finish it shortly, but I just got interrupted :).

Cheers!

weaverryan added a commit that referenced this pull requestAug 21, 2016
This PR was merged into the 2.8 branch.Discussion----------Tweaks to property info componentJust proofreading and making some tweaks to#5974Commits-------c9ea21f bad link!fcf5e69 fixing indentationbf5188f final tweaks7e296d8 [WIP] Tweaks to property info
@zanbaldwinzanbaldwin deleted the feature/component-property-info branchOctober 4, 2017 14:16
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@zanbaldwin@dunglas@TomasVotruba@wouterj@weaverryan@teohhanhui@dupuchba@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp