docs: minor edits for first version of driver manager docs #3180
Conversation
I don't think it adds anything here.
"essentially" makes it sound like there's more than two ways. Are there? If not, I think we can just say "there are two ways".
The option is technically in addition to the other option but it's clearer to say it's an alternative IMO.
Is this something I just don't know about or just a typo? Usually when I use LD_LIBRARY_PATH, I add absolute paths to a folder containing .so files and then I have to pass the basename of the file. Here, you say I'd pass 'adbc_driver' to load 'libadbc_driver.so'. Is that a typo? I also reworded the paragraph a bit.
Need empty lines before lists or these render in a broken fashion.
I don't think this needs to be a nested list item. It should just show up as a new paragraph in the parent list item
this was showing up as a code block which wasn't good
|
Hey @zeroshade, I didn't get a chance to review #3176 but I took a read and the doc looks great, with just a few minor editorial notes. I made each change as a separate commit so it's easier to track. Feel free to comment on any of them or ask me to revert. |
|
|
||
| Use ``adbc_driver(... , load_flags = adbc_load_flags())`` to pass options to the driver manager | ||
| Use ``adbc_driver(..., load_flags = adbc_load_flags())`` to pass options to the driver manager | ||
| regarding how to locate drivers specified by manifest. |
There was a problem hiding this comment.
We should either add a single space to each line here so it's in a quote box like the other tabs, or we should remove the single leading space from each line so it doesn't use a quote box.
Basically, all of the tabs should either line up with the : of :sync: or the s. We should be consistent
There was a problem hiding this comment.
Ahh, I see it now. Good eye.
Fixed in 13b741d.
There was a problem hiding this comment.
(They all render as not-quote-boxes, just normally now)
| * ``ADBC_CONFIG_PATH`` is a semicolon-separated list of directories to search for ``${name}.toml`` | ||
| ``ADBC_CONFIG_PATH`` is a semicolon-separated list of directories to search for ``${name}.toml`` |
There was a problem hiding this comment.
Same here, the * should stay, but remove the leading space so that the * is lined up with the I of If
|
Just a few minor comments. Thanks! |
This reverts commit f13a82a.
|
All comments are resolvable with my latest pushes. |
| * ``ADBC::LoadFlags::SEARCH_SYSTEM`` - search the system configuration directory | ||
| * ``ADBC::LoadFlags::ALLOW_RELATIVE_PATHS`` - allow a relative path to be provided | ||
| * ``ADBC::LoadFlags::DEFAULT`` - default value with all flags set | ||
| These can be provided by using ``ADBC::Database#load_flags=``. |
There was a problem hiding this comment.
you need a blank line before and after the bullet list
|
Spotted another ReST issue. |
|
🤞 I think I got all of the syntax issues and that this is ready for another look. |
zeroshade
left a comment
zeroshade
left a comment
There was a problem hiding this comment.
Looks good to me! I downloaded the archive and looked at it locally. Everything looks good 😄 Thanks again!
3 participants
Minor follow-up edits to #3176. Some syntax fixes, some editorial.