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

[Review] Added detailed Backwards Compatibility Promise text#3439

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 39 commits intosymfony:2.1fromwebmozart:bc-promise
Feb 20, 2014

Conversation

webmozart
Copy link
Contributor

I created a detailed text about which parts of the Symfony code are covered by our BC promise. The goals are to:

  • tell our users which parts exactly are safe to use
  • tell our contributors which changes they are allowed to do

I'll continue working on the individual rules of allowed changes. Nevertheless, let me hear your feedback.

Update (2014/01/10)

The draft is ready for review now.

Our Backwards Compatibility Promise
===================================

As of Symfony 2.1, we promise you backwards compatibility for all further 2.x
Copy link
Member

Choose a reason for hiding this comment

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

I would say2.3 here as this is more accurate.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Updated

Copy link
Member

Choose a reason for hiding this comment

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

then this should also be merged into 2.3

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I will open another PR when everything else is finished.

Change name No No
Reduce visibility Yes [1]_ No
Add parameter without a default value Yes [1]_ No
Add parameter with a default value Yes Yes
Copy link
Contributor

Choose a reason for hiding this comment

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

But "Add parameter with a default value" still breaks code if someone has overwritten the method in their class. Maybe also adding [1] here?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

You are right, thank you!

However, backwards compatibility comes in many different flavors. This page
lists which code changes are covered by our promise and to what degree.

If you are contributing to Symfony, make sure that your code changes comply to
Copy link
Contributor

Choose a reason for hiding this comment

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

I would perhaps say "....complywith the rules" rather than "comply to". Either that or "adhere to".

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thank you! Language suggestions are more than welcome.

however be documented in the UPGRADE file.

.. [2] Should be avoided. When done, this change must be documented in the
UGPRADE file.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: UPGRADE (apologies, I added this comment previously on an outdated diff)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

fixed

fabpot added a commit to symfony/symfony that referenced this pull requestJan 8, 2014
This PR was merged into the 2.5-dev branch.Discussion----------Upgrade File for 2.5Added upgrade info for#9601, as this pr might break code which overwrites this method and also to respectsymfony/symfony-docs#3439.Commits-------fefcf41 Added upgrade info for#9601
@fabpot
Copy link
Member

@webmozart The document looks very good to me; I've made some comments about minor changes.

Thank you so much for taking time to thoroughly document what BC means for us. I'm very happy with the result and it will make discussion about future changes so much easier.

@webmozart
Copy link
ContributorAuthor

Thanks@fabpot for the comments! I will adapt the document later today.

@webmozart
Copy link
ContributorAuthor

@fabpot I adapted the document now. Any more feedback?

Ensuring smooth upgrades of your projects is our first priority. That's why
we promise you backwards compatibility (BC) for all minor Symfony releases.
You probably recognize this strategy as `Semantic Versioning`_. In short,
Semantic Versioning means that only major releases (such as 2.0, 3.0 etc.) are
Copy link
Contributor

Choose a reason for hiding this comment

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

3.0, etc.

@fabpot
Copy link
Member

@webmozart Looks good to me

weaverryan added a commit that referenced this pull requestFeb 20, 2014
… text (webmozart)This PR was merged into the 2.1 branch.Discussion----------[Review] Added detailed Backwards Compatibility Promise textI created a detailed text about which parts of the Symfony code are covered by our BC promise. The goals are to:* tell our users which parts exactly are safe to use* tell our contributors which changes they are allowed to doI'll continue working on the individual rules of allowed changes. Nevertheless, let me hear your feedback.##### Update (2014/01/10)The draft is ready for review now.Commits-------0717192 Removed useless line breakce58ee9 Added rules for adding parent interfaces and moving methods/properties to parent interfaces/classesbe2251c Implemented@fabpot's comments90c4de6 Mentioned Semantic Versioning in the introduction2320906 Extracted duplicated text into _api_tagging.rst.incbdd3c03 Implemented changes suggested by@wouterjfd1d912 Typo11bb879 Grammar25443c0 Improved table formattinge11335f Improved the wording of the "Using Symfony" section4868452 Added prose about the difference between regular/API classes/interfaces8c6c7bf Simplified usage description6d9edf1 Improved wording: Changed "safe" to "guaranteed"ef1f021 Added note about test classes5a160c5 Added note about deprecated interfaces/classes69768dd Improved wording: use -> call, access0c6420f Added information about changing parameter types6501a35 Added information about changing return types that are classes or interfacesdfb3e8b Improved wordingbe76644 Added information about internal classes and interfacesaf3a645 Added note about requesting `@api` tagsdcbe79a Improved wordingefd3911 Added that adding custom properties is not safe00c6ebe Fixed safety statements54fd836 Language improvementsdb76288 Fixed headingsc6e850d Language fixes4c5a55d Rearranged page to have different sections for different user bases502ed95 Added: Some breaking changes to unsafe operations are documented in the UPGRADE file31ab2db Improved wordinga3ad08c Removed most of the "cannot" statements which are repeated in the tables now345410c Rearranged safe operations to make more senseafadaab Changed: The last parameters of a method may be removed44ecf16 Fixed: No parameters must be added (ever) to API class methods0e925cb Added tables with safe operations79ca9f7 Added information about type compatibilitydacd7ce Rearranged rules to be more easily understandable7320ed0 Updated BC promise to be valid as of Symfony 2.3840073c Added detailed BC promise text
@weaverryanweaverryan merged commit0717192 intosymfony:2.1Feb 20, 2014
@weaverryan
Copy link
Member

Nice work@webmozart - it reads very very well. And nice use of footnotes - I hadn't seen those before.

Cheers!

@webmozart
Copy link
ContributorAuthor

Thanks for merging! :)

Who's in charge of doing the styling of the docs? The footnotes could use some love :) Also, it would be nice if the links would be rendered as superscripted numbers, like in books.

I just realized we need to discuss one more thing, namely the changing of method return values inregular classes/interfaces (but not in API classes/interfaces). The current version allows (among others) to change the return values of Symfony methods from:

Original TypeTo New Type
arrayinstance ofArrayAccess,Traversable andCountable
ArrayAccessarray
Traversablearray
Countablearray

Contrary to the other changeslisted in the table, these changes might break applications:

Examples (rule 1):
$array =$object->getFoo();// fine if changed to an object$array[1] ='bar';foreach ($arrayas$key =>$value) {... }echocount($array);// breaks if changed to an objectarray_walk($array,'callback');// easy fix$array =iterator_to_array($object->getFoo());array_walk($array,'callback');
Examples (rule 2):
$array =$object->getFoo();// fine if changed to an array$array[1] ='bar';echo$array[0];// breaks if changed to an array$array->offsetSet(1,'baz');// easy fix$array =new \ArrayObject($object->getFoo());$array->offsetSet(1,'baz');

Rule 3 and 4 are basically identical to rule 2.

I originally allowed these changes because the upgrade path is so simple (iterator_to_array()/new \ArrayObject()), however do you agree with this?

A specific use case for these rules issymfony/symfony#9918.

@webmozartwebmozart deleted the bc-promise branchFebruary 20, 2014 21:04
@wouterj
Copy link
Member

Who's in charge of doing the styling of the docs? The footnotes could use some love :) Also, it would be nice if the links would be rendered as superscripted numbers, like in books.

ping@javiereguiluz

@javiereguiluz
Copy link
Member

@wouterj@webmozart I can take care of that. I'll return with more information when I know exactly when the new styles can be applied on the symfony.com website.

@webmozart
Copy link
ContributorAuthor

Thanks@javiereguiluz! You're the best! :)

@javiereguiluz
Copy link
Member

@wouterj@webmozart here it is the before/after comparison of the style of the footnotes. Please, tell me anything else that you'd like to change in this proposal and we'll see it in production soon:

footnotes

table_footnotes

@wouterj
Copy link
Member

Thanks@javiereguiluz for involving us in the process! :)

I like it, while I think the numbers on the footnote (first image,(1,2,3,4...)) should be somewhat smaller too, they are not that important imo

@javiereguiluz
Copy link
Member

@wouterj maybe we should hide those numbers? I don't know if they are useful at all. If you click on a footnote, there is no way to know which number you should go back.

@wouterj
Copy link
Member

@javiereguiluz yes, that's an option too. But please note that if the footnote only applies to one occurence, the footnote number will be a link instead. I think it's harder to remove that one?

@javiereguiluz
Copy link
Member

In the case of unique footnotes, we can disguise the footnote link simply by changing the font color.

@wouterj
Copy link
Member

Ok, let's do that :) (and also addcursor:default; to remove the pointer cursor ;) )

@webmozart
Copy link
ContributorAuthor

@javiereguiluz Awesome! Yes, I would like to hide the backlinks too ( (1,2,3,4,..) ) if that's possible with the RST parser. Is it possible to remove the squared brackets around the numbers?

@webmozart
Copy link
ContributorAuthor

Also, could you change the table style so that the grey horizontal lines don't cut through the black border of the table? Should be possible by moving the black border from the cells to the table itself.

@javiereguiluz
Copy link
Member

  • The backlink numbers are now hidden.
  • I've asked if we can remove the square brackets around footnote numbers, but I'm not very confident about being possible.
  • Definitely the styles of the tables need to be fixed. I haven't done it yet because we are postponing those style changes for the upcoming symfony.com redesign (we will announce some things about this soon).

@webmozart
Copy link
ContributorAuthor

Cool, thanks@javiereguiluz! I just realized that the fonts in the footnotes are very different. One is the font of the table content ([1]-[4]), the other is the one of the paragraphs in the normal page content ([5]-[6]). Is this waiting for the redesign too?

@javiereguiluz
Copy link
Member

@webmozart you are right! Font normalization is going to be one of the biggest tasks for the redesign.

@wouterj
Copy link
Member

@javiereguiluz you forgot to remove the link style for hover. You'll get a pointer cursor and an underline when hovering over the numbers in the footnotes.

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.

9 participants
@webmozart@Crell@fabpot@weaverryan@wouterj@javiereguiluz@richsage@danez@cordoval

[8]ページ先頭

©2009-2025 Movatter.jp