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

Improvements to registering an extension (#2)#3236

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 2 commits intosymfony:2.2fromflip111:patch-2
Dec 23, 2013
Merged

Improvements to registering an extension (#2)#3236

weaverryan merged 2 commits intosymfony:2.2fromflip111:patch-2
Dec 23, 2013

Conversation

flip111
Copy link
Contributor

QA
Doc fix?yes
New docs?no
Applies toall
Fixed tickets#9644 (symfony) and#3231 (symfony-docs)
  1. The original text is good but the topic of that paragraph is not "extension conventions" but "registering an extension" (i.e. if you follow these conventions then register goes like so ..)
  2. Cleaned up different terminology "Conventions" v "Standards" -> just conventions now
  3. Move this part to a more logical place. First create then register, then modify for additional needs.

I don't know why a comment about Symfony 2.1 (versionadded:: 2.1) is removed in the new documentation (master). I copied from there.

| Q             | A| ------------- | ---| Doc fix?      | yes| New docs?     | no| Applies to    | all| Fixed tickets |#9644 (symfony) and#3231 (symfony-docs)1. The original text is good but the topic of that paragraph is not "extension conventions" but "registering an extension" (i.e. if you follow these conventions then register goes like so ..)2. Cleaned up different terminology "Conventions" v "Standards" -> just conventions now3. Move this part to a more logical place. First create then register, then modify for additional needs.

* The extension should provide an XSD schema.

Manually registering an Extension class
Copy link
Member

Choose a reason for hiding this comment

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

Manually Registering an Extension Class

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

It Looks Pretty, CamelCase, But No We Don't Write English Like This

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

OK

@wouterj
Copy link
Member

+1 this is indeed much better!

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

To manually register an extension class override the
:method:`Bundle::build() <Symfony\\Component\\HttpKernel\\Bundle\\Bundle::build>`
Copy link
Member

Choose a reason for hiding this comment

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

:method:`Symfony\\Component\\HttpKernel\\Bundle\\Bundle::build`

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

This is outside of the scope of this PR (see improvements points 1,2 and 3). Please submit a new PR for this.

Copy link
Member

Choose a reason for hiding this comment

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

No it is not

@flip111
Copy link
ContributorAuthor

I'm not maintaining this PR anymore because of repo collab going out of scope.

@wouterj
Copy link
Member

Hi@flip111

I'm really sad you closing this pr... would be nice to get the changes in the docs.

From your comment, it looks like it was my fault? Could you please tell me what I did wrong? I and@xabbuh are reviewing almost all PRs, so it would be nice to learn how we can improve our reviewing comments.

Also, if you don't know how to fix the comments, don't heasitate (not sure if that's spelled correctly) asking us for help. We're always willing to help people contributing.

If yu really decide not to continue this, I'll create a new PR including your changes and a commit from me fixing the comments, so we can have this nice changes in the docs.

Thanks,
Wouter

@flip111
Copy link
ContributorAuthor

You closed the original PR because of "as there is nothing wrong with the docs and I can't see a way how we can rewrite it"

Then later when I submit this PR I get plenty of comments from you and xabbuh of all these little things beingWRONG. Some of which I didn't even write in the first place. Many of which are valid points such as too long lines and title CamelCase (but the reason was not stated and I didn't know about the rules).

So all in all it seems to jump on the first chance to analyse, critique and dismiss it.

Also taken the hassle for submitting documentation when not being routined. I don't feel like doing additional work to add in changes that were already in the documentation before this PR. And then having the chance of it not being merged anyway (as there was zero comment on the things I actually did want to improve).

But since you were so nice to ask why I closed it, I will reopen it and do a commit on the changes you suggested.

@flip111flip111 reopened thisNov 28, 2013
@wouterj
Copy link
Member

Hi, thanks for reopening!

I'll at some more explainations the next time, maybe my reviews become a little bit too robot-isch :)

The reason why we comment on small wrong things is because it's a lot easier to fix small things in the PR itself than after the merge. I spend a half year fixing many small things of PRs that were already merged, I want to avoid that in the feature.

And btw, I closed the precious issue as I thought anything was good, but as I alreadysaid in this PR, these changes are indeed better.

weaverryan added a commit that referenced this pull requestDec 23, 2013
Improvements to registering an extension (#2)
@weaverryanweaverryan merged commit7cb9c23 intosymfony:2.2Dec 23, 2013
weaverryan added a commit that referenced this pull requestDec 23, 2013
@weaverryan
Copy link
Member

Thanks for re-opening this@flip111 - the section header and re-ordering really does make a lot more sense. I appreciate it!

Cheers!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@flip111@wouterj@weaverryan@xabbuh

[8]ページ先頭

©2009-2025 Movatter.jp