-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Preview support #4782
Preview support #4782
Conversation
✔️ Deploy Preview for jenkins-io-site-pr ready!
😎 Browse the preview: https://deploy-preview-4782--jenkins-io-site-pr.netlify.app |
Insecure img urls:
|
Jenkinsfile_k8s
Outdated
"description": description, | ||
"required_contexts":[], | ||
"auto_merge":false, | ||
"transient_environment":false, |
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.
shouldn't this be true
?
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.
Damian says those urls never get deleted, so I marked it as false.
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.
if you don't do this any other PR deploying will set the deployment to inactive
https://docs.github.com/en/rest/reference/deployments#inactive-deployments
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.
Hrm. So i went with disabling that behavior, because its not transient URL. I don't know what the right behavior is though. This should prevent that issue though.
@MarkEWaite if we don't hear anything by end of week I'm just going to merge. I prefer the environments thingie, as its a bit quieter, but a comment saying the env is live is pretty easy too (just noisy). Ex jenkins-infra/plugin-site#951 (comment) |
Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>
Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>
@MarkEWaite I think its okay to squash and ship, infra.ci isn't 100% though, so if you do ship it, I recommend updating the checks to only care about ci.jenkins build not require infra ones yet. |
I've already done that it doesn't work, seems like a bug in GitHub |
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.
Interesting timing wise.
infra.ci is far quicker to build, maybe because it's not using containers for each part of building?
https://github.com/jenkins-infra/jenkins.io/pull/4782/checks?check_run_id=4724636018
Declarative: Checkout SCM (36 sec)
NPM Install (44 sec)
Bundle Install (37 sec)
Build (4 min 13 sec)
Deploy to preview site (1 min 8 sec)
vs
https://github.com/jenkins-infra/jenkins.io/pull/4782/checks?check_run_id=4724635636
Clean workspace (1.6 sec)
Checkout source (33 sec)
Build site (8 min 25 sec)
Archive site (27 sec)
Total time is 2 mins quicker for infra.ci, extra time added deploying to netlify but nowhere near as bad as plugin site
🤞🤞🤞
so this, while having a Jenkinsfile_k8s, doesn't actually build on infra.ci, so no Christmas present, but new years present!