-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
Updated the readme to include links to Jeckyll and a brief subsection… fixes #300 #301
base: main
Are you sure you want to change the base?
Conversation
… containing instructions for building a test site for use when updating the glossary.
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.
I've left some feedback. In general, I think this is good information to include.
### Building a local test site | ||
Once you have finished creating your glossary entry it may be useful to build a local test site to check the syntax and formatting is correct. To do this you will need a working installation of Jekyll on your local machine (instructions for which can be found [here](https://jekyllrb.com/docs/installation/). |
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.
### Building a local test site | |
Once you have finished creating your glossary entry it may be useful to build a local test site to check the syntax and formatting is correct. To do this you will need a working installation of Jekyll on your local machine (instructions for which can be found [here](https://jekyllrb.com/docs/installation/). | |
### Building a local test site (optional) | |
*N.B. this is not a required step for submitting a PR to Glosario.* | |
Once you have finished creating your glossary entry it may be useful to build a local test site to check the syntax and formatting is correct. To do this you will need a working installation of Jekyll on your local machine (instructions for which can be found [here](https://jekyllrb.com/docs/installation/). |
I understand the intention here, and I think providing a bit more information about how one might test the site is good. However, I'm concerned that this wording might make people feel as if they are expected to test the deployment before they submit a PR. We have been trying to make this project more inclusive and welcoming to people who may not know a lot about GitHub, local deployment, or other things. The submission of new entries offers a way to begin contributing to open-source projects that doesn't require a lot of previous experience.
As the person who is generally looking in on PRs to see why builds fail, I can attest that the error messages thrown up by the linter and other things can be quite arcane, and I do not want to place responsibility for that on newcomers. I do, however, let them know why things are failing, so that when they (hopefully) contribute again, they understand how to avoid the same errors.
|
||
With this, you can then use the command `make serve` this will create a local copy of the GitHub pages site that you can view in a web browser. The IP. address for this is listed in the command line output as `Server address: http://X.Y.Z:4000` however the default address is `http://localhost:4000`. |
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.
With this, you can then use the command `make serve` this will create a local copy of the GitHub pages site that you can view in a web browser. The IP. address for this is listed in the command line output as `Server address: http://X.Y.Z:4000` however the default address is `http://localhost:4000`. | |
With this, you can then use the command `make serve` this will create a local copy of the GitHub pages site that you can view in a web browser. The IP address for this is listed in the command line output as `Server address: http://X.Y.Z:4000` however the default address is `http://localhost:4000`. |
|
||
With this, you can then use the command `make serve` this will create a local copy of the GitHub pages site that you can view in a web browser. The IP. address for this is listed in the command line output as `Server address: http://X.Y.Z:4000` however the default address is `http://localhost:4000`. | ||
|
||
Another useful command is `make check` the just checks the syntax for the glossary is correct without actually creating the GitHub pages site. |
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.
Another useful command is `make check` the just checks the syntax for the glossary is correct without actually creating the GitHub pages site. | |
Another useful command is `make check` which checks the syntax for the glossary is correct without actually creating the GitHub pages site. |
Thanks for this @bjthorpe! I've left some comments and I'm requesting a review from @fmichonneau, as well, because I believe he has been more directly involved in the structure of the site. |
@@ -100,7 +100,12 @@ A glossary entry is structured like this: | |||
and the value may contain local links to other terms in this glossary | |||
(i.e., links starting with `#`) | |||
and/or links to outside sources. | |||
### Building a local test site |
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.
add new line before section
### Building a local test site | |
### Building a local test site |
@@ -100,7 +100,12 @@ A glossary entry is structured like this: | |||
and the value may contain local links to other terms in this glossary | |||
(i.e., links starting with `#`) | |||
and/or links to outside sources. | |||
### Building a local test site | |||
Once you have finished creating your glossary entry it may be useful to build a local test site to check the syntax and formatting is correct. To do this you will need a working installation of Jekyll on your local machine (instructions for which can be found [here](https://jekyllrb.com/docs/installation/). |
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.
wording change suggestion:
Once you have finished creating your glossary entry it may be useful to build a local test site to check the syntax and formatting is correct. To do this you will need a working installation of Jekyll on your local machine (instructions for which can be found [here](https://jekyllrb.com/docs/installation/). | |
Once you have finished creating your glossary entry, you could preview the site on your computer to check that the syntax and formatting are correct. To do this you will need a working installation of Jekyll on your local machine (instructions for which can be found [here](https://jekyllrb.com/docs/installation/). |
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.
I'm also wondering if it'd be worth pointing to The Carpentries lesson template setup instructions as it is more comprehensive than Jekyll's. The issue might be that it might be distracting/confusing for people not familiar with The Carpentries to understand the relationship between glosario and our lessons.
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.
Maybe we could take just the portion of that page that describes the Jekyll stuff and have it be one page in the Glosario site. That way we don't confuse people with stuff about the lesson, but we do show them where they can find more comprehensive instructions—if they are so inclined.
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.
yes that sounds good, especially given that we are going to change the content of this page in the near future as we're going to move away from Jekyll for our lessons.
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.
I've been trying to get this working, but I haven't managed to get the CSS cooperating. Glosario doesn't use an external theme, so I tried pulling the relevant bits from the Carpentries theme, but something about the code blocks isn't right—and I can't tell what, even though I've inspected multiple pages.
Is there some part of those code block boxes that is actually created by knitr that would somehow still be missing if I recapitulated the html myself? See images below for what it should look like, vs what it does look like.
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.
I've created a new branch (add_setup) where I've added a markdown version of the Jekyll setup from this page. I have replaced the knitr stuff with generic GitHub markdown code rendering. We can discuss more styling in the draft PR: #302.
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.
I think it's because we rely on bootstrap for the formatting and that it's missing here.
I agree with @baileythegreen: these changes look useful but the wording needs to be careful to ensure that it's not a necessary step for contribution. |
Closing this pull request since the last activity took place in April 2021. Please feel free to reopen in the future to make the necessary changes. |
I can take a look at this as I've gone through this process for the recent PR to adjust sorting. |
… containing instructions for building a test site for use when updating the glossary.
Fill in each of the sections (using NA for those that are not applicable).
Author:
Language:
Terms defined: