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.

Sign up for GitHub

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

Jump to bottom

[SITE] Adding new site + workflow #850

Merged
merged 5 commits into from
Nov 1, 2022
Merged

[SITE] Adding new site + workflow #850

merged 5 commits into from
Nov 1, 2022

Conversation

naskio
Copy link
Contributor

@naskio naskio commented Sep 14, 2022

  • Adding the Jekyll website inside Site/ folder.
  • Adding the workflow deploy-site.yml to deploy it to GitHub Pages automatically.

@naskio naskio changed the title [SITE ] Adding new site + workflow [SITE] Adding new site + workflow Sep 14, 2022
@naskio
Copy link
Contributor Author

naskio commented Sep 14, 2022

it is also required to add:

  • CNAME file
  • configure the site _config.yml to use the domain name ilgpu.net

m4rs-mt
m4rs-mt previously approved these changes Sep 22, 2022
View reviewed changes
Copy link
Owner

@m4rs-mt m4rs-mt left a comment

Choose a reason for hiding this comment

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

Fantastic work! I left a few minor comments that we might want to revisit before merging.

However, from my perspective, this PR is ready to go 🥇 !

Site/assets/css/site.scss Outdated Show resolved Hide resolved
Site/assets/js/search.js Outdated Show resolved Hide resolved
Site/scripts/generate_docs.py Outdated Show resolved Hide resolved
Site/assets/js/theme-switcher.js Outdated Show resolved Hide resolved
m4rs-mt
m4rs-mt reviewed Oct 31, 2022
View reviewed changes
Copy link
Owner

@m4rs-mt m4rs-mt left a comment

Choose a reason for hiding this comment

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

Looks good to me. However, there still seems to be a problem with the copyright headers. There is also a script to adjust the copyright headers in the Tools directory.

@naskio
Copy link
Contributor Author

naskio commented Nov 1, 2022

Looks good to me. However, there still seems to be a problem with the copyright headers. There is also a script to adjust the copyright headers in the Tools directory.

Ops, I had no clue about that. Thanks for your review.
It should be fixed now.

m4rs-mt
m4rs-mt approved these changes Nov 1, 2022
View reviewed changes
Copy link
Owner

@m4rs-mt m4rs-mt left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Special thanks also to @ericjohnsohnisc at this point, who redesigned the whole website in the first place 🥇

@m4rs-mt
Copy link
Owner

m4rs-mt commented Nov 1, 2022

it is also required to add:

  • CNAME file
  • configure the site _config.yml to use the domain name ilgpu.net

That's what we'll be dealing with offline.

@m4rs-mt m4rs-mt merged commit b4c59d9 into m4rs-mt:master Nov 1, 2022
@m4rs-mt m4rs-mt mentioned this pull request Nov 1, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m4rs-mt m4rs-mt m4rs-mt approved these changes

Assignees

@naskio naskio

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@naskio @m4rs-mt