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

CI: site generation failure should block PR merges #1357

Closed
StefanKarpinski opened this issue Aug 20, 2021 · 9 comments · Fixed by #1364
Closed

CI: site generation failure should block PR merges #1357

StefanKarpinski opened this issue Aug 20, 2021 · 9 comments · Fixed by #1364

Comments

@StefanKarpinski
Copy link
Member

https://www.julialang.org/blog/ is currently a 404

@logankilpatrick
Copy link
Member

Yep, just pushed a commit but not clear as to why

@StefanKarpinski
Copy link
Member Author

Locally, Franklin can't generate the site at all for me due to a syntax error in the JuliaCon highlights post.

StefanKarpinski added a commit that referenced this issue Aug 20, 2021
@logankilpatrick
Copy link
Member

Good catch. It really is the little things (:head-bang)

logankilpatrick added a commit that referenced this issue Aug 20, 2021
@rikhuijzer
Copy link
Contributor

tlienart/Franklin.jl#785

@tlienart
Copy link
Contributor

tlienart commented Aug 21, 2021

While I agree that Franklin should fail more explicitly and block the deployment, the current workflow to avoid this should be:

  1. Get stuff locally and test them locally, if it fails there it will definitely not deploy properly
  2. Check the preview, if you add new commits you'll have to do another PR as the preview doesn't get updated afaik
  3. Merge

@giordano
Copy link
Contributor

Preview doesn't work for pull requests from forks, which is the case for #1356. So at the moment we can't rely on always having the preview deployed

@StefanKarpinski StefanKarpinski changed the title the blog is gone CI: site generation failure should block PR merges Aug 25, 2021
@StefanKarpinski
Copy link
Member Author

I've changed the title to reflect that the blog is back but the root problem is that CI should prevent breaking the site by merging an apparently passing pull request.

@rikhuijzer
Copy link
Contributor

I'm grepping the logs for "Franklin Warning" and this works well for a year already (https://github.com/rikhuijzer/huijzer.xyz/blob/main/.github/workflows/CI.yml). That's the easy solution.

If you want to make things really reliable, fix tlienart/Franklin.jl#785 or write your own logic and tests to verify that Julia exits with exit code 1 when there is a problem in the Markdown/embedded Julia code.

@tlienart
Copy link
Contributor

@rikhuijzer would you mind maybe opening a PR here with your warning grepper? I'll eventually get to fixing 785 but not sure when and this seems pretty important :) thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants