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

update template version #17

Merged
merged 6 commits into from
Mar 24, 2023
Merged

update template version #17

merged 6 commits into from
Mar 24, 2023

Conversation

vincerubinetti
Copy link

@vincerubinetti vincerubinetti commented Mar 22, 2023

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for set-website ready!

Name Link
🔨 Latest commit 0c8233d
🔍 Latest deploy log https://app.netlify.com/sites/set-website/deploys/641dfdd2ff2cf1000702d27e
😎 Deploy Preview https://deploy-preview-17--set-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@d33bs
Copy link
Member

d33bs commented Mar 23, 2023

Hi @vincerubinetti, the new format from the preview looks amazing!

I have an in-progress review and wanted to ask for clarity: were there specific aspects of the changes we should focus on with this PR? I'm wondering this in reference to the upstream template dependencies, recognizing that certain elements may not be prudent to change within this instantiation of lab-website-template. Do you have thoughts / recommendations on this? Thank you in advance.

@vincerubinetti
Copy link
Author

You can make comments that relate to the template itself and i will respond and consolidate them into an upstream issue if needed.

Copy link
Member

@d33bs d33bs left a comment

Choose a reason for hiding this comment

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

Excited for this update, nice work on the updates! I left a few comments and suggestions with this review. Please don't hesitate to let me know if you have any questions. Apologies in advance for areas where I wasn't certain on the exact place to add a comment.

I'm marking this as approved as some of these may be more appropriate for changes to the template rather than this implementation. Those comments which are specific to this instance of the template I felt were minor and could possibly be addressed outside this PR as later changes (if it makes sense).

@vincerubinetti
Copy link
Author

@d33bs Mermaid and MathJax are now built into the template with Jekyll Spaceship. You may want to convert your blog posts to this simpler syntax (and remove the importing of the js libraries) in this PR.

@vincerubinetti vincerubinetti merged commit 73dd97f into main Mar 24, 2023
@vincerubinetti vincerubinetti deleted the update branch March 24, 2023 19:46
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 this pull request may close these issues.

3 participants