-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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: update ADR 59 #22423
docs: update ADR 59 #22423
Conversation
📝 WalkthroughWalkthroughThe pull request updates the document Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
docs/architecture/adr-059-test-scopes.md (3)
145-147
: Maintain consistent note block formatting.While the content accurately describes the transition to system tests, consider using consistent formatting for the two note blocks. The first note block uses three colons (
:::
) while maintaining the same content.-:::note +::: note End-to-end tests have been removed from v0.52+ and have been replaced fully by system tests. -::: +:::Also applies to: 213-215, 217-220
176-176
: Consider clarifying system tests vs E2E tests.While the table correctly shows the transition to system tests, it would be helpful to add a brief note explaining the key differences between system tests and the previous E2E tests approach, particularly regarding the testing environment and scope.
148-148
: Fix grammar and punctuation.Two minor issues need to be addressed:
- "End to end" should be hyphenated as "End-to-end" for consistency
- Add a comma after "Presently" for better readability
-End to end tests exercise the entire system +End-to-end tests exercise the entire system -Presently these tests are located +Presently, these tests are locatedAlso applies to: 150-150
🧰 Tools
🪛 LanguageTool
[grammar] ~148-~148: Did you mean the adjective “End-to-end” (spelled with hyphens)?
Context: ...en replaced fully by system tests. ::: End to end tests exercise the entire system as we ...(END_TO_END_HYPHEN)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
docs/architecture/adr-059-test-scopes.md
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/architecture/adr-059-test-scopes.md (1)
Pattern **/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/architecture/adr-059-test-scopes.md
[grammar] ~148-~148: Did you mean the adjective “End-to-end” (spelled with hyphens)?
Context: ...en replaced fully by system tests. ::: End to end tests exercise the entire system as we ...
(END_TO_END_HYPHEN)
[formatting] ~150-~150: Consider adding a comma after ‘Presently’ for more clarity.
Context: ...production environment as is practical. Presently these tests are located at [tests/e2e](...
(CONJUNCTIVE_LINKING_ADVERB_COMMA_PREMIUM)
🔇 Additional comments (1)
docs/architecture/adr-059-test-scopes.md (1)
8-8
: LGTM: Status and changelog updates are accurate.
The status change to "PROPOSED Implemented" and the new changelog entry appropriately reflect the complete migration from E2E tests to system tests.
Also applies to: 12-12
(cherry picked from commit e9436a6)
* main: (24 commits) build(deps): upgrade to [email protected] (#22436) docs: Update tendermint validators query pagination documentation (#22412) refactor(client/v2): offchain uses client/v2/factory (#22344) feat: wire new handlers to grpc (#22333) fix(x/group): proper address rendering in error (#22425) refactor(serevr/v2/cometbft): update `RegisterQueryHandlers` and GRPC queries (#22403) docs: update ADR 59 (#22423) build(deps): Bump github.com/fsnotify/fsnotify from 1.7.0 to 1.8.0 in /tools/cosmovisor (#22407) docs: Module account address documentation (#22289) feat(baseapp): add per message telemetry (#22175) docs: Update Twitter Links to X in Documentation (#22408) docs: redirect the remote generation page (#22404) refactor(serverv2): remove unused interface methods, honuor context (#22394) fix(server/v2): return ErrHelp (#22399) feat(indexer): implement `schema.HasModuleCodec` interface in the `bank` module (#22349) refactor(math): refactor ApproxRoot for readality (#22263) docs: fix KWallet Handbook (#22395) feat(client/v2): broadcast logic (#22282) refactor(server/v2): eager config loading (#22267) test(system): check feePayers signature (#22389) ...
Description
Replaces #22420
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit