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

splits CI build #2229

Merged
merged 2 commits into from
Sep 23, 2024
Merged

splits CI build #2229

merged 2 commits into from
Sep 23, 2024

Conversation

kdschlosser
Copy link
Contributor

@kdschlosser kdschlosser commented Sep 20, 2024

Splits the CI build into separate files and removes the need to decompress an archive of the HTML output. This also adds building using Windows as well.


📚 Documentation preview 📚: https://writethedocs-www--2229.org.readthedocs.build/

Copy link
Contributor

@plaindocs plaindocs 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 great to me.

@mxsasha want to put eyes on this before we merge it? tl;dr, I opened #2226 to add a Windows CI build to help keep WTD buildable on Windows. And @kdschlosser suggested splitting the build up, and did so, as well as adding in Windows.

@plaindocs
Copy link
Contributor

And those three waiting jobs are I assume from master branch, and will be removed when we merge this in.

@kdschlosser
Copy link
Contributor Author

They should disappear when this gets merged.

Like how everything now builds in parallel? a lot more efficient this way especially now that you have the Linux and the Windows builds which take some time to run.

@kdschlosser
Copy link
Contributor Author

It's also better having it split up because if you need to make a change to the CI and there is a problem that comes up it doesn't break everything. It only ends up breaking a small portion of it.

@plaindocs
Copy link
Contributor

Yep, love that last point. :-)

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Looks like a solid change to me 👍

@plaindocs
Copy link
Contributor

ok, with the second 👍 I'll merge this in.

@plaindocs plaindocs enabled auto-merge (squash) September 23, 2024 16:21
@plaindocs plaindocs merged commit 3eb2189 into writethedocs:main Sep 23, 2024
6 checks passed
@kdschlosser
Copy link
Contributor Author

Thanks guys. I believe this is definitely a step in the right direction since the CI is getting larger. The ability to have the CI run the tests and builds in parallel is the way to go.

@plaindocs plaindocs mentioned this pull request Sep 24, 2024
@plaindocs
Copy link
Contributor

For posterity, if you have specific rules mention in branch protection rules, you have to go update those after changing the CI names. 👍

@kdschlosser
Copy link
Contributor Author

we kind of touched base on that a bit when you asked about those 3 rules and them disappearing. I said they "should" and if they didn't then you would need to go into the actions and make adjustments as needed.

looks like you figured that out tho.

@plaindocs
Copy link
Contributor

Absolutely not pointing at you there ;-), I just read the first half and missed the second. All good.

@kdschlosser
Copy link
Contributor Author

no worries m8. This is a learning experience for us all, and that is the reason why we are doing this stuff. It's to learn and to share knowledge.

mxsasha added a commit that referenced this pull request Oct 7, 2024
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.

3 participants