Skip to content

Conversation

@Farhad-Shabani
Copy link
Contributor

@Farhad-Shabani Farhad-Shabani commented Mar 5, 2024

📖 Rendered

Part of: #1114

Implementation PR: #1115


PR author checklist:

  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@Farhad-Shabani Farhad-Shabani marked this pull request as ready for review March 5, 2024 21:30
Copy link
Member

@rnbguy rnbguy left a comment

Choose a reason for hiding this comment

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

LGTM 👍

One comment about listing future solutions for the negative consequences.

@Farhad-Shabani Farhad-Shabani merged commit e1f19af into main Mar 8, 2024
@Farhad-Shabani Farhad-Shabani deleted the farhad/adr010-enable-standolone-ics02-integration branch March 8, 2024 19:12
Farhad-Shabani added a commit that referenced this pull request Sep 9, 2024
* docs: ADR-010 enabling standalone ICS-02 integration

* imp: explaining why decode_client_state will be removed

* imp: use diff-formatted code snippets

* chore: put a comment for decode_client_state

* chore: apply suggestions from code review

Co-authored-by: Sean Chen <[email protected]>
Signed-off-by: Farhad Shabani <[email protected]>

* chore: apply suggestions from code review

Co-authored-by: Sean Chen <[email protected]>
Signed-off-by: Farhad Shabani <[email protected]>

* imp: update with details for host_consensus_state & validate_self_client

* nit: lint markdown

* nit: rename to HostClient/ConsensusState

* imp: update ADR with changes in implementation PR

* chore: back to the original implementation idea

* fix: adjust ClientStateDecoder

* nit

* nit: add comment for minimizing associated types

---------

Signed-off-by: Farhad Shabani <[email protected]>
Co-authored-by: Sean Chen <[email protected]>
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.

4 participants