Another documentation pass #77
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Revert back to old `format_list` logic, which was simpler * Fix `format_item` to not insert extra spaces after some commas, which resulted in incorrect API requests. * Use `vapply` rather than `sapply` in `format_list` to potentially catch weird cases and express expectations. * Add a few formatting tests.
Address a few bugs
brookslogan
reviewed
Apr 5, 2023
There was a problem hiding this comment.
Looks good in general. I've noted a lot of nit picks which are not blocking but which I think we'd want to address soon (e.g., maybe also in km-edit). I'm not sure I really gave this a good lookover; there are so many endpoints and it's hard to keep paying attention. Probably bad for maintainability. We might want to use some templates or the new support for inline computed strings in roxygen2.
|
Let me know if it's better to just merge & have me PR some changes for the above. |
|
Thanks for the close look! I'll make the changes you suggest, but yea, probably just start making the changes you want in another branch next time, I don't think we need to discuss the docs too deeply at this point. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I figured it would be faster for me to make the changes myself than to make comments in the other PR, so here is a documentation pass.
devinto the branch.