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

Allow setting a custom Carpentry type #585

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

milanmlft
Copy link
Contributor

Allow setting a custom value for the carpentry field in the config.yaml of a lesson.

At the moment, if a user wants to create a course website using the Carpentries workbench but with custom styling and logos, they would have to clone both the {sandpaper} and {varnish} repos:

  • {varnish} to modify the styling and add custom logos
  • {sandpaper} to modify the which_caprentry() and which_icon_carpentry() functions so that it can pick up the alternative logos from varnish.

See for example https://github.com/HealthBioscienceIDEAS/microscopy-novice, which uses the HealthBioscienceIDEAS/sandpaper and HealthBioscienceIDEAS/varnish forks.

This is rather cumbersome, as one needs to clone the entire {sandpaper} repo and then keep maintained, just to modify two lines of code.

With this PR, a user can use any value in the carpentry: config field and it will be picked up by {sandpaper} and used to get the matching logo from {varnish}, as long as there are logo files present in the {varnish} fork being used that (exactly) match the carpentry name. So this omits the need to fork {sandpaper}, only {varnish} needs to be forked.

Copy link
Member

@tobyhodges tobyhodges left a comment

Choose a reason for hiding this comment

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

This looks good to me, thank you @milanmlft. @froggleston @ErinBecker do you think we should add a test to ensure that the newly-defined default is being captured?

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.

None yet

2 participants