search

pydata / pydata-sphinx-theme

A clean, three-column Sphinx theme with Bootstrap for the PyData community
https://pydata-sphinx-theme.readthedocs.io
BSD 3-Clause "New" or "Revised" License
560 stars 303 forks source link

Add style linter to pre-commit, and run #1764

Closed gabalafou closed 1 month ago

gabalafou commented 3 months ago

Note: this did not pick up the bug fixed in #1761. Why? A selector of the form html .something (newline) html .something is not invalid. I'm not exactly sure what kind of linter rule one would write to pick up that kind of bug.

gabalafou commented 3 months ago

Should this PR be separated? One commit that adds stylelint, another that runs the linter?

gabalafou commented 2 months ago

I'm astonished the pseudo selector : vs :: worked !

The linter is being pedantic (as linters do). Browsers accept both :before (older standard)) and ::before (newer standard). See note on MDN ::before article.

jarrodmillman commented 2 months ago

@gabalafou Before merging this, it would be great if you could rebase and then add a third commit adding the commit corresponding to "fix errors emitted by style linter" to https://github.com/pydata/pydata-sphinx-theme/blob/main/.git-blame-ignore-revs

We will need to enable and use "Create a merge commit". Otherwise the commit in .git-blame-ignore-revs will be wrong.

gabalafou commented 2 months ago

@jarrodmillman will do, thanks for the tip!

drammock commented 2 months ago

@gabalafou Before merging this, it would be great if you could rebase and then add a third commit adding the commit corresponding to "fix errors emitted by style linter" to https://github.com/pydata/pydata-sphinx-theme/blob/main/.git-blame-ignore-revs

We will need to enable and use "Create a merge commit". Otherwise the commit in .git-blame-ignore-revs will be wrong.

@jarrodmillman since we typically squash-merge, I would suggest this workflow instead:

  1. (rebase and) merge this one edit: with just the addition of the linter, not the fixes, as suggested in https://github.com/pydata/pydata-sphinx-theme/pull/1764#issuecomment-2048633145
  2. do another PR that fixes the linter-discovered errors, and merge it
  3. do a third PR that adds the hash of (2) to .git-blame-ignore-revs

The advantage is that whoever does the merging doesn't need to have the repo-admin permissions to change our merge strategy, and doesn't need to remember to change it back afterwards.

gabalafou commented 1 month ago

This work is picked up in #1823.