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

Update instructions on publishing to GitHub pages #841

Merged
merged 2 commits into from
Oct 4, 2016

Conversation

Janpot
Copy link
Contributor

@Janpot Janpot commented Oct 4, 2016

fixes #501 #654

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Copy link
Contributor

@fson fson left a comment

Choose a reason for hiding this comment

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

Thanks @Janpot! I just tested this and it works great.

I just had a few comments about the text in teh readme. Could you fix those? I think this is ready to be merged after that.

git push -f origin gh-pages
git checkout -
```
To publish it at http://myname.github.io/myapp, run:
Copy link
Contributor

@fson fson Oct 4, 2016

Choose a reason for hiding this comment

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

The homepage field in package.json instructions a few lines above uses http://myusername.github.io/my-app as an example, let's use the same URL here.

Then run:

npm run deploy
```
Copy link
Contributor

@fson fson Oct 4, 2016

Choose a reason for hiding this comment

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

Can we only put the script and deployment command inside a code block? Now it looks a bit weird.

I mean like this (also changed the URL to http://myusername.github.io/my-app here):

To publish it at http://myusername.github.io/my-app, run:

npm install --save-dev gh-pages

Add the following script in your package.json:

 // ...
 "scripts": {
   // ...
   "deploy": "gh-pages -d build"
 }

Then run:

npm run deploy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Done

@fson
Copy link
Contributor

fson commented Oct 4, 2016

I just found out that gh-pages also overwrites the CNAME file (tschaub/gh-pages#127) so it looks like this won't fix #654 yet. However, I think this PR is a big improvement over the previous GitHub pages instructions, so we should take it anyway.

I found out that adding the CNAME file to public folder fixes the issue, so perhaps we should just document that as a fix #654. I think that can be done in a separate PR, however.

@gaearon
Copy link
Contributor

gaearon commented Oct 4, 2016

I found out that adding the CNAME file to public folder fixes the issue, so perhaps we should just document that as a fix #654.

👍

@fson fson merged commit feb6036 into facebook:master Oct 4, 2016
@fson
Copy link
Contributor

fson commented Oct 4, 2016

Thank you @Janpot!

@Janpot
Copy link
Contributor Author

Janpot commented Oct 4, 2016

@gaearon There's also an --add option in gh-pages that makes it only add files to the gh-pages branch and never remove.

@fson
Copy link
Contributor

fson commented Oct 4, 2016

But using that option would mean all the old builds are kept in the gh-pages branch, since they have a unique filename? I think adding the file in public folder would be a cleaner option in that case.

@Janpot
Copy link
Contributor Author

Janpot commented Oct 4, 2016

@fson Correct. Been skimming a bit more through their code and it seems their CLI has an option --remove that accepts a pattern of which files to remove
So I guess you could also do something like gh-pages --remove !CNAME -d build.
But the solution of public folder works just as well. It doesn't matter much I guess.

@Janpot Janpot deleted the gh-pages branch October 4, 2016 12:44
@gaearon
Copy link
Contributor

gaearon commented Oct 4, 2016

Can you post a final screenshot of what gets printed?

@fson
Copy link
Contributor

fson commented Oct 4, 2016

@Janpot yeah, looks like that's what the --remove option was added for: tschaub/gh-pages#75

I don't have a strong opinion on which way to use. I guess difference is that --remove allows you to configure the domain using GitHub settings UI, which is how it's documented in the official GitHub docs, but requires switching to this (slightly cryptic) deployment command. The other option requires you to add CNAME to public folder instead of changing the settings of the repo, but you can use the command without custom options.

@gaearon
Copy link
Contributor

gaearon commented Oct 4, 2016

Btw do we know for sure that this module works on windows?

@Janpot
Copy link
Contributor Author

Janpot commented Oct 4, 2016

@gaearon
screen shot 2016-10-04 at 15 20 36

Not sure whether it works on windows, I'm on a macbook. Looking at the amount of downloads in combination with the lack of any issues containing "windows" I'm assuming it does.

@gaearon
Copy link
Contributor

gaearon commented Oct 4, 2016

Looks great. Thanks!

@gaearon gaearon added this to the 0.7.0 milestone Oct 4, 2016
@gaearon
Copy link
Contributor

gaearon commented Oct 4, 2016

@fson Please tag with milestone before merging so we don't miss it in release notes

kitze added a commit to kitze/custom-react-scripts that referenced this pull request Oct 6, 2016
…react-app

# By Dan Abramov (2) and others
# Via Dan Abramov
* 'master' of https://github.com/facebookincubator/create-react-app:
  Correct a comment mistype in webpack production config (facebook#855)
  load setupTests file at setupTestFramework stage (facebook#846)
  Spell check (facebook#845)
  Tweak readme
  Update instructions on publishing to GitHub pages (facebook#841)
  Make webpackHotDevClient support webpack 2 too (facebook#840)
  Tweak eject output
  Promote "React must be in scope" to be an error (facebook#822)
  Fix script name to open chrome (facebook#831)
feiqitian pushed a commit to feiqitian/create-react-app that referenced this pull request Oct 25, 2016
* Update instructions on publishing to GitHub pages

* Update README formatting
maltestenzel pushed a commit to maltestenzel/custom-react-scripts that referenced this pull request Mar 7, 2018
…react-app

# By Dan Abramov (2) and others
# Via Dan Abramov
* 'master' of https://github.com/facebookincubator/create-react-app:
  Correct a comment mistype in webpack production config (facebook#855)
  load setupTests file at setupTestFramework stage (facebook#846)
  Spell check (facebook#845)
  Tweak readme
  Update instructions on publishing to GitHub pages (facebook#841)
  Make webpackHotDevClient support webpack 2 too (facebook#840)
  Tweak eject output
  Promote "React must be in scope" to be an error (facebook#822)
  Fix script name to open chrome (facebook#831)
roblingle pushed a commit to opensavannah/savannahbudget.party that referenced this pull request Sep 12, 2018
roblingle pushed a commit to opensavannah/savannahbudget.party that referenced this pull request Sep 29, 2018
roblingle pushed a commit to opensavannah/savannahbudget.party that referenced this pull request Sep 29, 2018
roblingle pushed a commit to opensavannah/savannahbudget.party that referenced this pull request Sep 29, 2018
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change GitHub hosting instructions
4 participants