-
Notifications
You must be signed in to change notification settings - Fork 3
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 downloadable comprehensive preview #61
Conversation
👀 Quick preview of website here Updated at 2024-03-12 with changes from 6714c80 |
No broken urls! 🎉 |
No spelling! 🎉 |
It works! |
Nice! this is helpful for checking the rendered version. Where is it getting archived? is it temporary for while the PR is open? |
👀 Quick preview of website here * * note not all html features will be properly displayed but it will give you a rough idea. Updated at 2024-03-12 with changes from 30220e5 |
👀 Quick preview of website here * * note not all html features will be properly displayed in the "quick preview" but it will give you a rough idea. Updated at 2024-03-13 with changes from the latest commit a3e10c4 |
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.
Minor comments! lgtm!
echo ::set-output name=website_link::$website_link | ||
echo ::set-output name=time::$(date +'%Y-%m-%d') | ||
echo ::set-output name=commit_id::$GITHUB_SHA | ||
docs_link=$(echo "https://github.com/$GITHUB_REPOSITORY/raw/preview-${{ github.event.pull_request.number }}/website-preview.zip") |
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.
can we name this something more descriptive, like "zip_file" ? docs_link makes it sound like its gonna take you to documentation
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.
Could we also break this chunk up into "# Store variables" and "# Print to log" for readability?
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.
Will do! On both accounts!
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.
Oh actually the "echo to log" can't really be separated the echo "zip_link=$docs_link" >> $GITHUB_OUTPUT
aren't echoing to log, the are saving the GitHub variables to the output and have to be done in the same step.
@@ -136,9 +141,11 @@ jobs: | |||
run: | | |||
course_name=$(head -n 1 _website.yml | cut -d'"' -f 2| tr " " "-") |
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.
course_name=$(head -n 1 _website.yml | cut -d'"' -f 2| tr " " "-") |
appears not to be used. Did you mean to echo it to the log?
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.
Deleted!
It is temporary while the PR is open! |
Summary
Was trying to think of a way that a more comprehensive preview could be shown. Because at times I want to check for a change that https://htmlpreview.github.io can't handle -- dropdowns and font-awesome icons for example don't get shown.
In this set up a rendered version of the website gets rendered as usually but a copy of it gets zipped up and archived so people could download it.