Skip to content
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

Misc. docs improvements #643

Merged
merged 5 commits into from
Sep 22, 2023
Merged

Misc. docs improvements #643

merged 5 commits into from
Sep 22, 2023

Conversation

acobster
Copy link
Contributor

SUMMARY

Fixes to various rough edges in the docs.

Checks

Docs only, no code changes.

whilo
whilo previously approved these changes Sep 21, 2023
Copy link
Member

@whilo whilo left a comment

Choose a reason for hiding this comment

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

Thank you! Overall looks good, feel free to address the minor comment I added if you have time.

@@ -37,11 +37,14 @@ $ datahike query '[:find ?n . :where [?e :name ?n]]' db:myconfig.edn
"Linus" # prints the name
```

Note that the `conn:<file>` argument to `transact` comes before the transaction
value(s), whereas the `db:<file>` argument to `query` comes after the query value,
Copy link
Member

Choose a reason for hiding this comment

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

The reason for this is that you can pass more than one db, in fact an arbitrary number of different data sources into the query engine. This is a core benefit of using datahike, so maybe we should also highlight that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I emphasized the connection by combining the two paragraphs. Let me know what you think. Also fixed a couple punctuation errors that I noticed.

Copy link
Member

Choose a reason for hiding this comment

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

You can pass also files with edn:/some/path/file.edn or json:/some/path/file.json. My idea in general is to keep the way inputs are represented medium specific, while the API should always be the same. For a command line client it was necessary to provide a small DSL for file loading and dereferencing.

This is also mentioned here:
https://github.com/replikativ/datahike/blob/main/src/datahike/cli.clj#L20

Copy link
Member

Choose a reason for hiding this comment

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

As a sidenote: Files are fairly reasonable as an abstraction on Unix, but maybe there are better ways to design this interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm interesting. Well now that you got me thinking about it, one thing this DSL rules out is using Bash process substitution to specify file contents ad-hoc, like:

datahike transact <(echo '{:store {:backend :mem :id "mydb"}}') ...

I don't know when I would use that over a regular file, and if you wanted to support multiple formats with it you would need to pair it with an option or something (datahike transact --edn <(...)). Just thinking out loud, since you mention Unix and files.

Copy link
Member

Choose a reason for hiding this comment

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

Transact does support to read in transaction data from STDIN already, but that is only for transact. I need to think about it more. I think you could pipe through files, right?

Copy link
Member

@whilo whilo left a comment

Choose a reason for hiding this comment

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

Thanks so much for these fixes!

@whilo whilo merged commit 4df1ff4 into replikativ:main Sep 22, 2023
@whilo
Copy link
Member

whilo commented Sep 22, 2023

@acobster I decided to merge this for now and we should discuss further improvements in follow up issues or on slack if you are down.

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.

2 participants