8000 MAINT Remove unused parametrize in `test_figure_rst_srcset` by lucyleeow · Pull Request #1172 · sphinx-gallery/sphinx-gallery · GitHub
[go: up one dir, main page]

Skip to content

MAINT Remove unused parametrize in test_figure_rst_srcset #1172

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

Merged
merged 5 commits into from
Aug 18, 2023

Conversation

lucyleeow
Copy link
Contributor
@lucyleeow lucyleeow commented Aug 17, 2023

Also removes unused parametrization in test_figure_rst_srcset, and move private funcs to above their use.

Originally visit_srcset_other was here to check adding other builders see: #808 (comment), but not used so removed here.

@@ -190,6 +176,28 @@ def _parse_srcset(st):
return srcset


def _copy_images(self, node):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this back down to minimize the diff? It helps with git blame

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah diff is ugly, I'll revert.

@lucyleeow lucyleeow changed the title MAINT Remove unused functions in directives.py MAINT Remove unused parametrize in test_figure_rst_srcset Aug 18, 2023
@lucyleeow
Copy link
Contributor Author
lucyleeow commented Aug 18, 2023

I think the removal of unused functions was done in #1169 , I might have done that by mistake since lint doesn't pass if we don't remove. This now just fixes the test.

@larsoner larsoner merged commit b927883 into sphinx-gallery:master Aug 18, 2023
@larsoner
Copy link
Contributor

Thanks @lucyleeow !

@lucyleeow lucyleeow deleted the fix_srcset branch August 18, 2023 11:46
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
0