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

Migrate docs infrastructure from Make to Nox #377

Merged
merged 4 commits into from
Sep 4, 2024

Conversation

CAM-Gerlach
Copy link
Member

Pull Request

Pull Request Checklist

  • Read and followed this repo's Contributing Guidelines
  • Based your PR on the latest version of the correct branch (master or 4.x)
  • Checked your writing carefully for correct English spelling, grammar, etc
  • Described your changes and the motivation for them below

Description of Changes

Our automation infrastructure still uses a bunch of old Makefiles, including the soft-deprecated Sphinx ones and our own in the root, that don't work cross-platform, are arduous to edit and don't provide Python-specific facilities of dedicated tools. Therefore, this PR ports our existing Makefile functionality (and quite a bit more) to Nox, updates the Readme/Contributing guide and CIs accordingly, and removes the old ones.

@CAM-Gerlach CAM-Gerlach self-assigned this Sep 3, 2024
@CAM-Gerlach CAM-Gerlach force-pushed the nox-migration branch 2 times, most recently from 75a4f24 to 3e1dcba Compare September 3, 2024 04:07
@CAM-Gerlach
Copy link
Member Author

This is ready for review; the one remaining blocker is a just-emerged issue, RJWadley/stylelint-no-unsupported-browser-features#299 in stylelint-no-unsupported-browser-features due to a change in the CanIUse data it consumes dynamically from the web, that causes Stylelint proper to crash.

Seems its likely to be fixed shortly as a PR by the maintainers that fixes it, anandthakker/doiuse#191 , is already open, but it would also block merge of the translations infra PR if it isn't fixed by then, so we can wait until I have that up and then just disable that plugin temporarily if its still an issue by the time its hard-blocking.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @CAM-Gerlach!

@ccordoba12
Copy link
Member

ccordoba12 commented Sep 3, 2024

so we can wait until I have that up and then just disable that plugin temporarily if its still an issue by the time its hard-blocking.

I think that's not necessary because stylelint runs a js/css lint check and this PR hasn't changed that. So please disable it so this one can be merged later today.

You can enable it again in your next PR, which will probably change and/or add js/css for the new translations dropdown.

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Sep 4, 2024

I think that's not necessary because stylelint runs a js/css lint check and this PR hasn't changed that. So please disable it so this one can be merged later today.

That's not why I was suggesting delaying it, but rather to just avoid the messyness of disabling a check in an unrelated PR, and then remembering it to enable it again in another unrelated PR. Also, it turns out that we're going to need to fix the broken links in the docs, which does involve some non-trivial CSS changes to add the Linux icon to the download buttons.

However, after reading the issue again, it might actually be workaroundable by pinning a specific transitive sub-dependency with pre-commit, which is not so bad for now and now that we have to do it in the link PR anyway, that makes more sense since it's all about fixing links and things that rotted recently.

You can enable it again in your next PR, which will probably change and/or add js/css for the new translations dropdown.

As we've discussed many times by now :) , my next PR will not be adding the translation dropdown, but first adding the translation automation infrastructure that is a prerequisite, to automate extracting the translations, interfacing with Crowdin and building with them (and actually doing so).

@CAM-Gerlach CAM-Gerlach merged commit 76c446a into spyder-ide:master Sep 4, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants