Skip to content

Rewrite the chapter about traversing pages#50

Merged
stof merged 2 commits intominkphp:masterfrom
stof:pages
Mar 7, 2015
Merged

Rewrite the chapter about traversing pages#50
stof merged 2 commits intominkphp:masterfrom
stof:pages

Conversation

@stof
Copy link
Copy Markdown
Member

@stof stof commented Feb 4, 2015

Closes #36

@stof
Copy link
Copy Markdown
Member Author

stof commented Feb 4, 2015

@aik099 I would appreciate a review on this one

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Maybe we should use table notation here or list or unordered list (ul/li) like it was done before: *``id_or_name``- description.
  2. The id and id_or_name selectors are only available since Mink 1.6

I guess we should mentioned version number, when particular feature was introduced and when it will be deprecated in the documentation, like it's done in Symfony docs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is using a definition list in the output (<dl>). IMO, it looks much more readable than a table here.

Regarding version where it has been added, I agree for Mink 1.7+ features. However, I'm not sure it makes sense to add it for features which in Mink 1.6 (it has been released 5 months ago, and there was no uptodate doc for 1.4 or 1.5 at all). What do you think

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess many people still use Mink 1.5 and when they see in documentation the id_or_name, then it would be useful to know, that they should upgrade to Mink 1.6 to use it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, we have lots of changes between Mink 1.5 and 1.6 in the way function works (because they were not defined in a consistent way in Mink 1.5 because of the crappy driver tests). So making the doc highlight the changes from Mink 1.5 to 1.6 might make our job harder (and we are already missing lots of doc)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not talking about all documentation. Just where new functionality (not bugfixes) were introduced in 1.6 version we need to highlight that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The versionadded directive is not styled by the RTD theme currently. See readthedocs/sphinx_rtd_theme#99
So using it currently looks ugly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wow. I wasn't aware, that there is a special directive for that. I was just thinking about something like some feature .... (since 1.5) text.

@aik099
Copy link
Copy Markdown
Member

aik099 commented Feb 7, 2015

Today’s review completed.

@stof
Copy link
Copy Markdown
Member Author

stof commented Feb 21, 2015

@aik099 updated

@stof
Copy link
Copy Markdown
Member Author

stof commented Mar 7, 2015

OK, merging this as you haven't made other comments.

stof added a commit that referenced this pull request Mar 7, 2015
Rewrite the chapter about traversing pages
@stof stof merged commit 46ad986 into minkphp:master Mar 7, 2015
@stof stof deleted the pages branch March 7, 2015 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rewrite the page traversing chapter

2 participants