Skip to content

Conversation

@rjrudin
Copy link
Contributor

@rjrudin rjrudin commented Jul 12, 2023

I'm going to do docs in a separate PR, after all the metadata stuff is supported too.

There are only a lot of new files here because I needed them in the test app to test out things like optimistic locking and temporal writes.

I'm going to do docs in a separate PR, after all the metadata stuff is supported too. 

There are only a lot of new files here because I needed them in the test app to test out things like optimistic locking and temporal writes.
@rjrudin rjrudin force-pushed the feature/496-write-docs branch from a3f4e66 to bbcf2b9 Compare July 12, 2023 21:47
self,
uri: str,
content,
content_type: str = None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One thought on this design - we should be able to reuse this class for reading documents - i.e. getting back an array of Document objects. But the args starting with content_type are all specific to writing a set of documents, and they won't be populated when reading a document. We could capture that in the docstring for the constructor, but I think they're still potentially confusing in that they're only for writing a document.

An alternative would be to use **kwargs and clearly document what keyword args can be passed in based on what you plan to do with a Document. That might be worth changing to after doing the "Read documents" story.

@rjrudin rjrudin merged commit 1e1ad80 into develop Jul 13, 2023
@rjrudin rjrudin deleted the feature/496-write-docs branch July 13, 2023 18:52
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.

3 participants