search

carpentries / styles

Styles for The Carpentries lessons. No README to avoid merge conflicts with lessons. Demo 👇
https://carpentries.github.io/lesson-example
Other
84 stars 94 forks source link

Template workflow: add two more lessons #584

Closed maxim-belkin closed 3 years ago

maxim-belkin commented 3 years ago

I propose to add 2 more lessons to our template-testing matrix: carpentries/lesson-example and datacarpentry/astronomy-python. The first one is probably one of the lessons that the first-time maintainers look at pretty regularly. The latter one uses The Carpentries remote theme, which makes it a unique lesson in the build matrix.

ErinBecker commented 3 years ago

Two thumbs way way up to including astronomy-python in the testing matrix. @rudyphd and I have been trying to get that lesson to stop failing checks for the past two weeks and failing . . . .

maxim-belkin commented 3 years ago

The "Test template" action doesn't run make lesson-check or make lesson-check-all, so it wouldn't tell us if there are any issues with these checks for the tested lessons. Technically, we can add make lesson-check-all to the "Test template" action because it doesn't affect the final status of the action. But it'll come at a cost of about a minute of GitHub Action time per pull request. I'm not sure how much GH Action time The Carpentries have but I assume 2000 minutes -- perhaps someone could confirm this here: https://github.com/organizations/carpentries/settings/billing (I don't have access to this page).

zkamvar commented 3 years ago

The "Test template" action doesn't run make lesson-check or make lesson-check-all, so it wouldn't tell us if there are any issues with these checks for the tested lessons. Technically, we can add make lesson-check-all to the "Test template" action because it doesn't affect the final status of the action. But it'll come at a cost of about a minute of GitHub Action time per pull request. I'm not sure how much GH Action time The Carpentries have but I assume 2000 minutes -- perhaps someone could confirm this here: https://github.com/organizations/carpentries/settings/billing (I don't have access to this page).

It's 3000 minutes, but that's for private repos. It's currently unlimited for public repos.

Given that, I think make lesson-check-all would definitely be good to add.

zkamvar commented 3 years ago

That being said, would you like me to add the change to your branch?

maxim-belkin commented 3 years ago

Sure

maxim-belkin commented 3 years ago

I think we should add it as a separate step... what do you think?

zkamvar commented 3 years ago

I think we should add it as a separate step... what do you think?

Yes, it makes much more sense to do that so that we avoid mixing up the outputs of the two commands

zkamvar commented 3 years ago

Right now, the only one that's failing is Astronomy because of the switch to the remote theme (which needs to update the build tools separately).

maxim-belkin commented 3 years ago

the only one that's failing is Astronomy

This should've been fixed in carpentries/styles#585. I'll try closing and reopening this PR because force-pushes don't seem to help.

maxim-belkin commented 3 years ago

OK, so once https://github.com/datacarpentry/astronomy-python/pull/28 is merged, the (DC) Foundations of Astronomical Data Science (Linux) check should pass

maxim-belkin commented 3 years ago

All checks have passed 1 skipped and 11 successful checks

🎉

zkamvar commented 3 years ago

Thank you so much for this, @maxim-belkin! Yay for more robust integration tests and yay for finally getting GHA to work!

I will merge this now!

fmichonneau commented 3 years ago

Thank you everyone for this!