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

Updated the readme to include links to Jeckyll and a brief subsection… fixes #300 #301

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ this project implements a glossary of terms used in data science and data engine

1. The master copy of the glossary lives in `glossary.yml`.
Its format is described below.
1. This file is turned into a single-page GitHub Pages site using Jekyll.
1. This file is turned into a single-page GitHub Pages site using [Jekyll](https://jekyllrb.com/docs/).
1. It is also turned into a [Python package](https://github.com/carpentries/glosario-py) called `glosario`
and an [R package](https://github.com/carpentries/glosario-r) with the same name.

Expand Down Expand Up @@ -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
Copy link
Contributor

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

Suggested change
### Building a local test site
### 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/).
Comment on lines +103 to +104
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### 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/).

@bjthorpe

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

wording change suggestion:

Suggested change
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/).

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Screen Shot 2021-04-07 at 21 33 11

Screen Shot 2021-04-07 at 21 29 24

Copy link
Contributor

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.

Copy link
Contributor

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.


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`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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.

## Open issues

1. Should we provide one function for interactive definition lookup
Expand Down
Loading