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

STYLE: Bump style changes #154

Merged
merged 25 commits into from
May 13, 2021
Merged

STYLE: Bump style changes #154

merged 25 commits into from
May 13, 2021

Conversation

jhlegarreta
Copy link
Contributor

Bump style changes.

maxim-belkin and others added 18 commits April 21, 2021 10:13
bin/lesson_check.py: one more fix for using_remote_theme()
Current syncing procedure that used in the Template workflow fails for:

1. Lessons that are, in fact, nsync with the styles repo.
2. For lessons that use The Carpentries' remote theme and have deleted
   some of the files.

This PR makes this step a little bit more intelligent and takes into
account the above two scenarios.
Co-authored-by: Zhian N. Kamvar <[email protected]>
Template workflow: smarter syncing with the styles repo
bin/lesson_check.py: allow comments and empty lines in links.md

Fixes carpentries/styles#581
bin/lesson_check.py: use proper function
Allow lines that contain a single image or a single link
to go over the suggested line length limit.
bin/lesson_check.py: allow exceptions to line length limit

Fixes carpentries/styles#591
@jhlegarreta jhlegarreta added the type:formatting Formatting needs to be fixed label May 7, 2021
@jhlegarreta jhlegarreta requested review from kaitj and josephmje May 7, 2021 22:39
@netlify
Copy link

netlify bot commented May 7, 2021

Deploy preview for carpentries-dmri ready!

Built with commit 5c8cb2d

https://deploy-preview-154--carpentries-dmri.netlify.app

maxim-belkin and others added 3 commits May 9, 2021 02:50
This change hardens the pattern that matches single-line
image or link:

1. It extends the pattern to be matched in a heading
2. It allows the line to contain {: ...} customizations
3. It allows the line to end with \
lesson_check.py: harden single-line image/link pattern

Fixes carpentries/styles#591
@josephmje
Copy link
Contributor

Thanks @jhlegarreta! Before merging, I just want to confirm whether the _includes, _layouts and assets folders should be included in the sync (see here).

@jhlegarreta jhlegarreta marked this pull request as draft May 10, 2021 14:43
@jhlegarreta
Copy link
Contributor Author

jhlegarreta commented May 10, 2021

Thanks @jhlegarreta! Before merging, I just want to confirm whether the _includes, _layouts and assets folders should be included in the sync (see here).

Thanks for caring about those.

Also, it is not ready to be merged yet, see carpentries/styles#597

This PR allows up to 3 non-word (`\W` in Python's `re`-speak) characters
in the beginning and end of the pattern that matches links and images.
This is to allow lesson developers place punctuation marks, parentheses,
or other symbols before or after the link or image on the same line in
Markdown.
lesson_check.py: relax P_LINK_IMAGE_LINE pattern
@jhlegarreta jhlegarreta marked this pull request as ready for review May 12, 2021 14:22
@jhlegarreta
Copy link
Contributor Author

Thanks @jhlegarreta! Before merging, I just want to confirm whether the _includes, _layouts and assets folders should be included in the sync (see here).

Thanks for caring about those.

@josephmje please comment about the above folders when you have the information. If those files are to be excluded, maybe the relevant Carpentries repositories/materials should explicitly tell about it.

@josephmje
Copy link
Contributor

I got some clarification from @zkamvar in the Carpentries slack. I'll post his comments below:

Michael:
Would anyone happen to know the difference between the carpentries' styles repo and the carpentries-theme repo? To me, it seems like most of the development is happening in styles, which later ends up getting synced with carpentries-theme. However, the theme repo also seems to have some unique files that add to the way incubator lessons get displayed

Zhian:
I have a bit more context for this:
The mix of remote and non-remote themes are largely due to a combination of a rapid expansion of lessons on top of a model that was meant for a handful (the styles repo), the historical lack of support for remote themes on GitHub (until well into the expansion phase), and the delay between scale and additional staff to support development and maintenance (that's me!).
It's worth noting that the lesson infrastructure has gone through a lot of shifts historically (raw HTML -> pandoc -> Jekyll), and in the early days, those shifts have sometimes been rapid (https://software-carpentry.org/blog/2015/07/pushing-back.html).
The shift to remote themes occurred because it was seen as a way to modernize the infrastructure and make it "easier" to maintain the changes since lessons that use the remote theme would rely on a single source of truth for the styling, but it made a manual dependency chain longer and more difficult because the tools for building the lessons were embedded within the repository.
To be clear, we are absolutely moving away from this model as we described in the design principles for the next iteration of the lesson template (which is currently in alpha testing and should have a release candidate by the end of the summer): https://carpentries.org/blog/2020/08/lesson-template-design/

The maintenance chain right now is styles -> carpentries-theme -> incubator-theme

Michael:
Thanks for the thorough explanation Zhian! I really appreciate it. That makes a lot of sense to me. From reading the blog post, it looks like the future goal is to remove styling elements from the lesson repository. So me trying to automate syncing with the styles repo is probably not necessary at this point

Zhian:
Correct. Plus, I'll be working on the automation process this week. There were a few false starts earlier due to the shifting sands of GitHub workflow permissions (which I'm glad they have finally been locked down).

@josephmje
Copy link
Contributor

josephmje commented May 12, 2021

For this lesson, since we are using the carpentries-theme repo as a remote, it's fine to remove the _includes, _layouts and assets folders since those will be checked upstream.

We should also remove any existing files we have in those folders and only keep _includes/links.md and _includes/lesson_links.md since those files have been modified from the theme repo.

Remove non-custom files dwelling in upstream `carpentries-theme`.
@jhlegarreta
Copy link
Contributor Author

@josephmje done in 5c8cb2d .

@josephmje josephmje merged commit 01fdca7 into carpentries-incubator:main May 13, 2021
@jhlegarreta jhlegarreta deleted the BumpStyle branch May 13, 2021 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:formatting Formatting needs to be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants