-
Notifications
You must be signed in to change notification settings - Fork 94
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
Add automatic deployment preview of PRs when changing the site #658
Conversation
Signed-off-by: Jirka Kremser <[email protected]>
That means we won't be able to correct the discovered layout issues within the same PR, so the proposed solution doesn't address the need for constant fast feedback during authoring.
This token is a standard GitHub token with a Also, my understanding is Netlify acts like a bot, as one of the PR checks. What happens if the Netlify account is deleted, locked, or if the available monthly build minutes threshold is exceeded? Will it be possible to still proceed with PR merge? |
It's not perfect, but that's kind of how I would use it anyway. Working on web related change locally, verifying it on the wrt JEKYLL_GITHUB_TOKEN imho a token from one of the maintainers can't/should not be used here. The reason is that GH doesn't provide a way to control the RBAC in a per repo base (it's not granular enough, + doesn't support read only on repo). So if I used mine or your token w/
Definitely, bus-factor == 1 is always bad, I'll invite you all once it's merged (it can't be part of the PR :D)
It's an optional thing running externally in async fashion. If their servers are down, the limit was reached or anything bad happened, then it just wont put that comment on the PR that's all. |
@@ -0,0 +1,7 @@ | |||
[build] | |||
command = "git config remote.origin.url || git remote add -f -t gh-pages origin https://github.com/k8gb-io/k8gb && git fetch origin gh-pages:gh-pages && git checkout gh-pages && make netlify-build" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
command is too long and complex. Can be moved to small sh
script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can, I was thinking about it, but I didn't want to pollute the / of the repo with netlify specific stuff. I can change that if you think, it's better. Well, it will be more readable definitely. In that case we probably don't need the Makefile target on gh-pages
branch at all. The script can be quite dangerous, you know changing branches, doing checkouts, adding remotes, but I can also add the big if $CI=true
conditions, or similar flag that Netlify has
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build-web.sh
/ netlify.sh
/ .netlify.sh
? I like the last one..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this at all? does netlify erase git metadata?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get your question @k0da :) The command is not about git metadata, it's about telling netlify how to build the website. The remote called origin is not set in their environment (no idea how they clone the repo, it's not part of the logs) so we need to add it, then fetching also the gh-pages branch (again not fetched by them), then basically simulating this: https://github.com/k8gb-io/k8gb/blob/master/.github/workflows/gh-pages.yaml#L21:L24 (part is done here in this one liner and the other part on the gh-pages' Makefile -> not ideal, merging it to 1 bash script)
This PR should go in only after #657
Add config for netlify that will trigger the build and deploys the preview of pull-request.
Such PR needs to be opened against master and some of the following files need to be changed:
{README,CONTRIBUTING,CHANGELOG}.md docs/
Example:
this is example PR on my fork w/ a change that doesn't contain the web page related change: test jkremser/k8gb#8
and this is an example PR w/ web related change: cool change jkremser/k8gb#9
Some limitations:
It's a one shot thing so even if new commits go to the PR or if the pr is closed and opened it wont re-trigger the build.
We have 300mins of free build time each month and I've used 20 during the development (1 build is like a 1 or 2 minutes) it should be more than enough
Also the website required GH token to be able to fill some metadata about the repo -
JEKYLL_GITHUB_TOKEN
. Fortunately, it uses the thing only in read only mode probably to prevent the rate limiter blocks. I've set this token in the Netlify web and used one that doesn't have write access to this repo w/ the minimal privileges. It's a token from my wife actually who has access to 1 private repo w/ her web site :D So even if it leaks, no biggie.If it works well, some future improvement could be running this thing also if the PR is opened against
gh-pages
(change to layouts, CSS, etc.)Signed-off-by: Jirka Kremser [email protected]