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

Documentation Re-org #115

Merged
merged 17 commits into from
Jul 20, 2023
Merged

Documentation Re-org #115

merged 17 commits into from
Jul 20, 2023

Conversation

winder
Copy link
Contributor

@winder winder commented Jul 6, 2023

Summary

There is a lot of good content in the docs, but they are spread out a little too much, and the organization is overly influenced by some limitations of the developer portal website.

This PR is intended to re-organizes and re-write the documentation.

Some of the major changes:

  1. The top level README is now the GettingStarted page.
  2. The Configuration page does not have enough content to justify its existence and has been removed.

Test Plan

N/A

@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #115 (8f6e343) into master (442791a) will increase coverage by 2.61%.
The diff coverage is 76.23%.

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
+ Coverage   67.66%   70.27%   +2.61%     
==========================================
  Files          32       36       +4     
  Lines        1976     2493     +517     
==========================================
+ Hits         1337     1752     +415     
- Misses        570      646      +76     
- Partials       69       95      +26     
Impacted Files Coverage Δ
conduit/data/block_export_data.go 100.00% <ø> (+92.30%) ⬆️
conduit/metrics/metrics.go 100.00% <ø> (ø)
conduit/pipeline/metadata.go 69.11% <ø> (ø)
...duit/plugins/exporters/filewriter/file_exporter.go 81.63% <ø> (-1.06%) ⬇️
conduit/plugins/exporters/postgresql/util/prune.go 78.43% <ø> (ø)
conduit/plugins/importers/algod/metrics.go 100.00% <ø> (ø)
...gins/processors/filterprocessor/fields/searcher.go 77.50% <ø> (ø)
...ins/processors/filterprocessor/filter_processor.go 83.82% <ø> (+3.54%) ⬆️
...plugins/processors/filterprocessor/gen/generate.go 34.28% <ø> (ø)
conduit/plugins/processors/noop/noop_processor.go 64.70% <ø> (+6.81%) ⬆️
... and 16 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

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

My usual nit regarding adding a line between the end of each paragraph and the following code block plus a question regarding whether we should resolve the introduced TODO's in this PR.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated
Comment on lines 89 to 90
Once configured, start Conduit with your data directory:
```sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Once configured, start Conduit with your data directory:
```sh
Once configured, start Conduit with your data directory:
```sh

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
winder and others added 2 commits July 10, 2023 11:39
Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>
README.md Outdated Show resolved Hide resolved
@winder winder marked this pull request as ready for review July 19, 2023 19:22
@winder winder requested review from algoanne and tzaffi July 19, 2023 20:09
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
noop - noop exporter
postgresql - Exporter for writing data to a postgresql instance.
```
First we need to make sure we have Conduit installed. Head over to the installation section of [the README](../../README.md) for more details. For this tutorial we'll assume `conduit` is installed and available on the path.
Copy link
Contributor

Choose a reason for hiding this comment

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

just noticing that one of our two tutorials recommends going to the installation section of the README for install instructions (this one) and the other one just tells you how to do it from the binary. Should we try to be consistent across all our tutorials?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like "IndexerWriter.md" directs people to the release page for a download.

we can again use the list command. For example,
`file_writer` exporter.
To get more details about each of these individually, and their configuration variables,
use the list command or check the [README](../../conduit/plugins/exporters/filewriter/). For example:
Copy link
Contributor

Choose a reason for hiding this comment

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

is this the right link? Maybe it should be to the top page of the plugins? And also maybe we should call it "plugins readme" or something?

Copy link
Contributor Author

@winder winder Jul 20, 2023

Choose a reason for hiding this comment

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

Suggested change
use the list command or check the [README](../../conduit/plugins/exporters/filewriter/). For example:
use the list command or check the [plugins README](../../conduit/plugins/). For example:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to reference the plugin TOC. I guess it could have links to each of the plugins instead.

docs/tutorials/WritingBlocksToFile.md Outdated Show resolved Hide resolved
Now we will fill in the proper fields for these. I've got a local instance of algod running at `127.0.0.1:8080`,
with an API token of `e36c01fc77e490f23e61899c0c22c6390d0fff1443af2c95d056dc5ce4e61302`. If you need help setting up
Now we will fill in the algod importer configuration. I've got a local instance of algod running at `127.0.0.1:8080`,
with an API token of `e36c01fc77e490f23e61899c0c22c6390d0fff1443af2c95d056dc5ce4e61302` and aan admin API token of `2d9ceaf47debb8aff77c62475b8337f6ecef75c2aa8aa02170a8d38b9126b75a`. Find these in the `algod.token` and `algod.admin.token` files from your algod data directory. If you need help setting up
algod, you can take a look at the [go-algorand docs](https://github.com/algorand/go-algorand#getting-started) or our
[developer portal](https://developer.algorand.org/).
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm I'm just thinking about how to help people get algod set up properly for conduit. In the other tutorial (Indexer Writer one) we do just walk them through how to do it. I wonder if that snippet could be included in the algod importer plugin documentation page, and then we could point to it from anywhere? Because there are specifics for the importer (in particular the follow mode) that people need to know. Here they wouldn't get that information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, this link is not very useful to someone new.

Co-authored-by: algoanne <90275860+algoanne@users.noreply.github.com>
Copy link
Contributor

@algoanne algoanne left a comment

Choose a reason for hiding this comment

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

I think there's still work to be done, but everything in this PR is a major improvement. We can follow-up with further updates in future PRs.

@winder winder merged commit bddfb21 into algorand:master Jul 20, 2023
3 checks passed
@winder winder deleted the will/doc-updates branch July 20, 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