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

lint: Capture Bible chapters followed by comma #749

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

erinendrei
Copy link
Contributor

Allows [Chapter Name], <span epub:type="z3998:roman>...</span> to be flagged by lint as needing modernization, which currently does not happen. My last production contained a case of this.

@acabal
Copy link
Member

acabal commented Sep 12, 2024

Thanks! Can you add a test case for this in the test framework?

@acabal acabal merged commit 8f53960 into standardebooks:master Sep 13, 2024
1 check passed
@acabal
Copy link
Member

acabal commented Sep 13, 2024

Excellent, thanks!

@erinendrei
Copy link
Contributor Author

erinendrei commented Sep 13, 2024

OK, I just pushed a test you can review, but I'm having some difficulties with empty golden files being generated.

I had to delete the following lines from the in chapter file's <head> in order for lint to successfully run, because the relevant css files don't exist. Other test data files include these lines without a test css directory existing either (e.g. t-010, t-074). Strangely (to me) pytest does generate the correct non-empty golden file for t-060 when I delete the following lines, but not for t-010 and t-074; the golden files are empty. Are you able to say what I'm missing here?

<link href="../css/core.css" rel="stylesheet" type="text/css"/>
<link href="../css/local.css" rel="stylesheet" type="text/css"/>

Edit: I didn't see the message/closure of PR before posting this comment

@acabal
Copy link
Member

acabal commented Sep 13, 2024

It seems to be working fine for me, it generates the golden files without creating changes in the repo, so that's all we need.

@erinendrei
Copy link
Contributor Author

OK thanks, it must be something for me locally.

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.

2 participants