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

Fixes for github deploy #1774

Merged
merged 10 commits into from
Jul 19, 2018
Merged

Fixes for github deploy #1774

merged 10 commits into from
Jul 19, 2018

Conversation

paulmelnikow
Copy link
Member

@paulmelnikow paulmelnikow commented Jul 12, 2018

This addresses some long-standing comments in #1458 (comment)

We should also adopt Gatsby, create-react-app, or something similar designed for static sites, to eliminate the unnecessary runtime dependency on Next.js while also letting someone else maintain our front-end tooling. :-D These alternative tools might work just fine in subdirectories without config, and we might be able to leave Jekyll turned on (though we don’t need it). However these git-related changes are orthogonal.

  • Don’t check out master, making it possible to deploy the currently checked-out commit
  • Disable Jekyll which we don’t need. This allows _next folders to be deployed, and the related URL rewriting to be removed.
  • Set NEXT_ASSET_PREFIX so the site works at both shields.io and badges.github.io/shields
    • This was possible in my github account, but doesn't seem to be possible in the badges account.
  • Completely empty the deploy branch’s index before deployment. This prevents errors from broken symlinks, while preserving the commit history in the deploy branch.
  • Do the deployment work in a git working tree. This requires Git 2.18 but makes it possible to do the above very safely.

Closes #1784

This addresses some long-standing comments in #1458 (comment)

We should also adopt Gatsby, create-react-app, or something similar designed for static sites, to eliminate the unnecessary runtime dependency on Next.js while also letting someone else maintain our front-end tooling. :-D These tools might work just fine in subdirectories without config, and we might be able to leave Jekyll turned on (though we don’t need it). However these git-related changes are orthogonal.

- Don’t check out master, making it possible to deploy the currently checked-out commit
- Disable Jekyll which we don’t need. This allows _next folders to be deployed, and the related URL rewriting to be removed.
- Set NEXT_ASSET_PREFIX so the site works at both shields.io and badges.github.io/shields
- Completely empty the deploy branch’s index before deployment. This prevents errors from broken symlinks, while preserving the commit history in the deploy branch.
- Do the deployment work in a git working tree. This requires Git 2.18 but makes it possible to do the above very safely.
@shields-ci
Copy link

shields-ci commented Jul 12, 2018

Messages
📖

✨ Thanks for your contribution to Shields, @paulmelnikow!

Generated by 🚫 dangerJS

@paulmelnikow paulmelnikow added the operations Hosting, monitoring, and reliability for the production badge servers label Jul 14, 2018
@paulmelnikow
Copy link
Member Author

Would like to get this addressed for #1783. Could I entreat a review?

@RedSparr0w
Copy link
Member

I'll give it a go, not sure how well this pc will handle it though.

@paulmelnikow
Copy link
Member Author

Thanks! Just reading it would be okay. If you do decide to run it, please do that on a fork so it doesn’t deploy something in the process.

Copy link
Member

@RedSparr0w RedSparr0w left a comment

Choose a reason for hiding this comment

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

Changes look good 👍

Makefile Outdated
@@ -7,7 +7,7 @@ all: website favicon test
favicon:
node lib/badge-cli.js '' '' '#bada55' .png > favicon.png
Copy link
Member

@RedSparr0w RedSparr0w Jul 18, 2018

Choose a reason for hiding this comment

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

Is it just me, or does this no longer produce anything?
Also without the .png it produces this for me:

I presume it's supposed to make this though:

https://img.shields.io/:--bada55.png?style=plastic

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh, weird. node lib/badge-cli.js '' '' '#bada55' > favicon.svg produces the right svg for me. No quotes… wonder if that could be a windows thing? Is it better if '' '' is replaced with "" "" in the makefile?

However the png is not working for me either. It makes a valid PNG but it's all gray. I upgraded imagemagick, and it changed to a different gray (!) but still isn't working correctly.

Copy link
Member Author

@paulmelnikow paulmelnikow Jul 19, 2018

Choose a reason for hiding this comment

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

Opened a new issue: #1788. Going to just back out the favicon updating for now, since we still have a working one checked in.

@paulmelnikow paulmelnikow merged commit e39b280 into badges:master Jul 19, 2018
@shields-deployment
Copy link

This pull request was merged to master branch. Now this change is waiting for deployment.
Deploys usually happen every few weeks. After deployment changes are copied to gh-pages branch.

This badge displays deployment status:

@paulmelnikow paulmelnikow deleted the gh-deploy-fixes branch July 19, 2018 02:46
git -C ${DEPLOY_TEMP} commit -m '[DEPLOY] Completely clean the index'
cp -r build/* ${DEPLOY_TEMP}
cp favicon.png ${DEPLOY_TEMP}
echo shields.io > ${DEPLOY_TEMP}/CNAME
Copy link
Member

@espadrine espadrine Jul 22, 2018

Choose a reason for hiding this comment

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

@paulmelnikow The point of the CNAME file was to have GitHub redirect from badges.github.io/shields to shields.io.

I believe it no longer needs that, as it is now set up through the project settings: https://help.github.com/articles/adding-or-removing-a-custom-domain-for-your-github-pages-site/

Copy link
Member Author

Choose a reason for hiding this comment

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

Something in my testing suggested the file still has an effect. When I ran this script on my fork without the CNAME file in gh-pages, it turned off the custom domain.

This link suggests changes made in the UI are stored in the repo: https://help.github.com/articles/troubleshooting-custom-domains/#github-repository-setup-errors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
operations Hosting, monitoring, and reliability for the production badge servers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants