-
Notifications
You must be signed in to change notification settings - Fork 82
Indent code blocks to make it part of ordered list #1570
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @abhro, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a documentation formatting issue by adjusting the indentation of Julia code blocks within ordered lists. The goal is to improve the rendering of these sections, making the code blocks appear as sub-elements of their respective list items, thereby enhancing readability and adherence to common Markdown interpretation, despite potential rendering quirks in the immediate build environment. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request aims to fix a Markdown rendering issue in the PairwiseFusion docstring by indenting code blocks to make them part of an ordered list. The change correctly applies a 4-space indent to the code blocks, which is standard practice. While this is a step in the right direction, the complete fix likely requires also correcting inconsistent indentation in the paragraphs of the list items themselves, as noted in the detailed comment.
| ```julia | ||
| y = x[1] | ||
| for i in 1:N | ||
| y = connection(x[i + 1], layers[i](y)) | ||
| end | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indenting this code block by 4 spaces is the correct approach to associate it with the list item according to Markdown standards, and is consistent with other docstrings in this file (e.g., RepeatedLayer).
However, as you've noted, this may not fully resolve the rendering issue. The problem likely lies in the inconsistent indentation of the paragraph preceding this block. On line 390, the text is indented by 3 spaces, while the list item on line 389 effectively starts its content at column 5 (after 1.).
To fix the list rendering completely, the paragraph on line 390 should also be indented by 4 spaces to align with the text on line 389. While that line isn't part of this change, adjusting it should make your indentation of this code block work as expected.
It doesn't work. The rendered list is still fragmented, with a single item list, then a code block, then another single item list, then another code block. However, the second list does start at 2 as opposed to 1 in the current build. Unsure of the build intricacies involved here. But for most Markdown parsers, it should appear as one list.
5bc9a35 to
193794e
Compare
Note: the fix in this commit doesn't even work. The rendered list is still fragmented, with a single item list, then a code block, then another single item list, then another code block.
However, the second list does start at 2 as opposed to 1 in the current build. Unsure of the build intricacies involved here. But for most Markdown parsers, it should appear as one list.