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

docs(indexer): more clarity for ChainIndexer configuration setup #12701

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Nov 19, 2024

Feedback already suggesting that our lack of automagic config switching is leading to some confusion. I'd still rather avoid automatically turning on ChainIndexer in these cases so we'll try with more clear error messages and see if we continue to get reports.

@rvagg rvagg added the skip/changelog This change does not require CHANGELOG.md update label Nov 19, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

@rvagg rvagg changed the title doc(indexer): more clarity for ChainIndexer configuration setup docs(indexer): more clarity for ChainIndexer configuration setup Nov 19, 2024
@github-actions github-actions bot dismissed their stale review November 19, 2024 00:36

PR title now matches the required format.

Copy link
Member

@virajbhartiya virajbhartiya left a comment

Choose a reason for hiding this comment

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

I am not sure but will it be a good idea to add a commented-out config for chainidnexer the way EnableEthRPC = true is set to true but commented in the config file by default so that it is easier for a new user.

@rjan90
Copy link
Contributor

rjan90 commented Nov 19, 2024

Needs a make gen/make docsgen it seems, but apart from that, lgtm.

@rvagg
Copy link
Member Author

rvagg commented Nov 19, 2024

commented-out config for chainidnexer

unfortunately the default config is generated from settings automatically, and their values are meant to reflect what they are currently set to - so they are a kind of documentation for the user, demonstrating what the current settings are, so they have to change them if they want them.

@rvagg
Copy link
Member Author

rvagg commented Nov 19, 2024

ahh, actually needs a docsgen-cli because of the config file output, gen and docsgen don't cut it (I ran them already!)

@rvagg rvagg force-pushed the fix/chainindexer-config-doc branch from 45d5db3 to 787fa23 Compare November 19, 2024 07:45
@rvagg rvagg enabled auto-merge (squash) November 19, 2024 07:45
@rvagg rvagg merged commit 1f5f51f into master Nov 19, 2024
83 checks passed
@rvagg rvagg deleted the fix/chainindexer-config-doc branch November 19, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does not require CHANGELOG.md update
Projects
Status: 🎉 Done
Development

Successfully merging this pull request may close these issues.

4 participants