Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the demo-warning. #5683

Closed
wants to merge 4 commits into from
Closed

Improve the demo-warning. #5683

wants to merge 4 commits into from

Conversation

GuGuss
Copy link

@GuGuss GuGuss commented Sep 9, 2015

We need a link to Platform.sh and a very small explanation of it.

@xabbuh
Copy link
Member

xabbuh commented Sep 9, 2015

To me, the way it is shown is totally fine. 👍

<a href="http://symfony.com/doc/current/{{ pagename }}">Visit on symfony.com</a>.</p>
<h4>Pull request build</h4>
<p>Each pull request of the Symfony Documentation is automatically deployed and hosted on <a href="https://platform.sh">Platform.sh</a>.<br>
Visit the page on <a href="http://symfony.com/doc/current/{{ pagename }}">symfony.com</a>.</p>
Copy link
Member

Choose a reason for hiding this comment

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

I think we should link to the HTTPS docs here.

@GuGuss
Copy link
Author

GuGuss commented Sep 10, 2015

thanks @xabbuh for your review.

@GuGuss
Copy link
Author

GuGuss commented Sep 18, 2015

👍
@xabbuh Hi, any idea when that'll be merged? Thanks.

@xabbuh
Copy link
Member

xabbuh commented Sep 23, 2015

@weaverryan @wouterj @javiereguiluz Any opinion on this one?

@GuGuss I would like to a 👍 from another docs team member before merging here.

@javiereguiluz
Copy link
Member

I like the reworded message and I think it's fair to add the link to platform.sh. My only concern is about the "Visit the page on symfony.com" message. I'm not sure if its intention is clear enough and by the way, it won't work when the page is new (which I know it's not a very frequent scenario).

Anyway, my concern can be also applied to the original message, so I'm 👍 about merging these changes.

@xabbuh
Copy link
Member

xabbuh commented Sep 23, 2015

I agree that we can handle this in a different pull request if we feel the need to do so.

@xabbuh
Copy link
Member

xabbuh commented Sep 23, 2015

Thank you Augustin.

xabbuh added a commit that referenced this pull request Sep 23, 2015
This PR was submitted for the 2.8 branch but it was merged into the 2.3 branch instead (closes #5683).

Discussion
----------

Improve the demo-warning.

We need a link to Platform.sh and a very small explanation of it.

Commits
-------

74ce3e1 Improve the demo-warning.
@xabbuh xabbuh closed this Sep 23, 2015
@GuGuss
Copy link
Author

GuGuss commented Sep 24, 2015

@xabbuh @javiereguiluz you guys rock as reviewers! I wish there were more community projects like this one!

@wouterj
Copy link
Member

wouterj commented Sep 24, 2015

@javiereguiluz I added the "view this page on symfony.com" for 2 reasons: (a) People on the demo docs that just wanted to read the doc page can quickly navigate to the doc page and (b) I've used it a couple of times already to quickly create a visual diff (e.g. when someone fixes some syntax and I want to quicly check if it really did fix anything)

@javiereguiluz
Copy link
Member

@wouterj I agree and like this feature :) My concern is about the text of the link. The original was "Visit on symfony.com" and the new one is "Visit the page on symfony.com" I think that the text you included in your comment is much better: "View this page on symfony.com"

weaverryan added a commit that referenced this pull request Sep 26, 2015
@weaverryan
Copy link
Member

I've just updated the language

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants