Skip to content

Conversation

@hramos
Copy link
Contributor

@hramos hramos commented Aug 1, 2016

We've been getting a lot of documentation PRs opened against 0.29-stable, 0.30-stable, and so on, instead of master. This is because our doc site is also based on RN release cuts, so clicking on the "Edit on GitHub" links on a document will take you to the markdown source for that release branch instead of the latest doc on master.

See #9095 for an example of such a PR.

In this PR we edit the link to say View on GitHub. Though it may not prevent PRs from being opened against a release branch, removing the "Edit" CTA may help in this regard.

@ghost
Copy link

ghost commented Aug 1, 2016

By analyzing the blame information on this pull request, we identified @vjeux and @easyCZ to be potential reviewers.

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Aug 1, 2016
@vjeux
Copy link
Contributor

vjeux commented Aug 1, 2016

The goal is to get people to send updates to the docs. Can we change the url to point to master instead?

@hramos
Copy link
Contributor Author

hramos commented Aug 1, 2016

@vjeux sure, we can do that

@vjeux
Copy link
Contributor

vjeux commented Aug 1, 2016

If you want some inspiration to update it, I really like the way flow displays it (at the very bottom) https://flowtype.org/docs/getting-started.html#_

@hramos
Copy link
Contributor Author

hramos commented Aug 1, 2016

PTAL

@ghost
Copy link

ghost commented Aug 1, 2016

@hramos updated the pull request.

@ghost
Copy link

ghost commented Aug 1, 2016

@hramos updated the pull request.

@hramos
Copy link
Contributor Author

hramos commented Aug 1, 2016

Link to master is now at the bottom, with a "edit this page on GitHub" CTA

var React = require('React');

function getGitHubPath(path) {
return [
Copy link
Contributor

Choose a reason for hiding this comment

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

seems overkill to use an array and join where you could use +

@vjeux
Copy link
Contributor

vjeux commented Aug 1, 2016

I just realized that we have HeaderWithGithub because a same page is combined from different places. For example the Text page http://facebook.github.io/react-native/docs/text.html has 4 different Edit on Github links. I'm not sure we can only have a single one like flow does without bigger thoughts on this.

What do you think?

@ghost
Copy link

ghost commented Aug 2, 2016

@hramos updated the pull request.

@facebook-github-bot
Copy link
Contributor

@hramos updated the pull request.

@hramos
Copy link
Contributor Author

hramos commented Aug 2, 2016

@vjeux good catch on the headers. I've settled on using a combination of both, which seems to work well IMHO.

screencapture-localhost-8079-react-native-docs-text-html-1470173650696

@hramos hramos changed the title Rename Edit on GitHub to View on GitHub Update Edit on GitHub links to point to master Aug 2, 2016
@facebook-github-bot
Copy link
Contributor

@hramos updated the pull request.

1 similar comment
@ghost
Copy link

ghost commented Aug 2, 2016

@hramos updated the pull request.

@vjeux
Copy link
Contributor

vjeux commented Aug 2, 2016

lgtm :)

}

.edit-page-block {
padding: 5px;
Copy link
Contributor

Choose a reason for hiding this comment

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

please use 2 spaces

@vjeux
Copy link
Contributor

vjeux commented Aug 2, 2016

lgtm :)

@facebook-github-bot
Copy link
Contributor

@hramos updated the pull request.

@hramos
Copy link
Contributor Author

hramos commented Aug 3, 2016

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

I will not rubber stamp and land your change for you @hramos! I can import it for you and you can get your change reviewed by someone though :)

@hramos
Copy link
Contributor Author

hramos commented Aug 3, 2016

OK fine @facebook-github-bot :) @vjeux can you rubber stamp?

@vjeux
Copy link
Contributor

vjeux commented Aug 3, 2016

@facebook-github-bot shipit

@ghost ghost added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Aug 3, 2016
@ghost
Copy link

ghost commented Aug 3, 2016

Thanks for importing.

@ghost ghost closed this in 01cb1e4 Aug 4, 2016
JoelMarcey added a commit that referenced this pull request Aug 5, 2016
Cherry-pick #9149 into 0.31-stable branch
bubblesunyum pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
Summary:
We've been getting a lot of documentation PRs opened against `0.29-stable`, `0.30-stable`, and so on, instead of `master`. This is because our doc site is also based on RN release cuts, so clicking on the "Edit on GitHub" links on a document will take you to the markdown source for that release branch instead of the latest doc on `master`.

See facebook#9095 for an example of such a PR.

In this PR we edit the link to say View on GitHub. Though it may not prevent PRs from being opened against a release branch, removing the "Edit" CTA may help in this regard.
Closes facebook#9149

Differential Revision: D3664368

Pulled By: vjeux

fbshipit-source-id: 395c0813f736bfbe1be4b4fb1182f9060169365d
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
Summary:
We've been getting a lot of documentation PRs opened against `0.29-stable`, `0.30-stable`, and so on, instead of `master`. This is because our doc site is also based on RN release cuts, so clicking on the "Edit on GitHub" links on a document will take you to the markdown source for that release branch instead of the latest doc on `master`.

See facebook#9095 for an example of such a PR.

In this PR we edit the link to say View on GitHub. Though it may not prevent PRs from being opened against a release branch, removing the "Edit" CTA may help in this regard.
Closes facebook#9149

Differential Revision: D3664368

Pulled By: vjeux

fbshipit-source-id: 395c0813f736bfbe1be4b4fb1182f9060169365d
@javache javache deleted the edit-on-github branch December 7, 2016 23:35
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants