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.

Sign up for GitHub

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

Jump to bottom

simibc: improves package documentation for simibc #662

Merged
merged 25 commits into from
Feb 7, 2023
Merged

simibc: improves package documentation for simibc #662

merged 25 commits into from
Feb 7, 2023

Conversation

danwt
Copy link
Contributor

@danwt danwt commented Jan 12, 2023

Description

Adds a README, doc.go and many more and improved docstrings to testutils/simibc. There are no functional changes or public API changes.

Please note that this is the first of improvements to be made, but another PR (#658) is using the public API, so I want to wait until that is moved before making any functional changes.

Also, I want to add an example test showing in a simpl way how things can be used, but I want to tackle some higher priority things before I do that.

Type of change

If you've checked more than one of the first three boxes, consider splitting this PR into multiple PRs!

  • Refactor: Changes existing code style, naming, structure, etc.
  • Docs: Adds documentation

shaspitz
shaspitz approved these changes Jan 23, 2023
View reviewed changes
Copy link
Contributor

@shaspitz shaspitz left a comment

Choose a reason for hiding this comment

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

Comments all seem very useful, and nothing seems like it's regressing diff test functionality. LGTM!

@mpoke mpoke requested a review from MSalopek January 23, 2023 18:04
@sainoe
Copy link
Contributor

sainoe commented Feb 3, 2023

Let's merge this after #692 since it's a lower priority

shaspitz
shaspitz reviewed Feb 6, 2023
View reviewed changes
tests/difference/core/model/src/traceUtil.ts
@@ -32,7 +32,7 @@ const meta = {
// Commit of interchain-security/ that the trace was generated.
commit: childProcess.execSync('git rev-parse HEAD').toString().trim(),
// Diff between the working tree and the commit.
diff: childProcess.execSync('git diff').toString().trim(),
// diff: childProcess.execSync('git diff').toString().trim(),
Copy link
Contributor

Choose a reason for hiding this comment

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

@sainoe @MSalopek do we want to comment this out?

Copy link
Contributor

Choose a reason for hiding this comment

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

Once that's decided, we're probably good to merge this one

Copy link
Contributor

Choose a reason for hiding this comment

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

That will include the full git diff output at the header of the traces.json.

It does seem like a nice feature, but I don't see a purpose of including the full diff inside a trace file. Commit hash should be enough and can be used to obtain a diff.

@shaspitz shaspitz merged commit f866815 into main Feb 7, 2023
@shaspitz shaspitz deleted the danwt/simibc-clarity-improvements branch February 7, 2023 16:06
ThanhNhann pushed a commit to decentrio/interchain-security that referenced this pull request Jan 3, 2024
@ramacarlucho @colin-axner
@seantking @dependabot @damiannolan @crodriguezvega @tac0turtle @fedekunze
fix: support custom chain IDs for testing (cosmos#774)
8dfbc9c 
* Backport commits from main to v3 release branch (cosmos#682)

* reorganize channel handshake handler (cosmos#647)

* reorganize channel handshake handler

split out channel state changes into its own function.

* readjust 27-interchain-accounts to not rely on state being set before the application callback

* add changelog and migration doc entry

* Update modules/core/04-channel/keeper/handshake.go

* docs: ICA Overview (cosmos#626)

* docs: ica overview

* fix: ordering

* add spacing

* fix: spacing

* fix: remove bulletpoints

* fix: wording

* update go mod for security vulnerabilities (cosmos#655)

* update go mod for security vulnerabilities

* update package-lock.json

* update vue dependency (cosmos#662)

* bump glob-parent version in json package (cosmos#663)

* build(deps): bump actions/setup-go from 2.1.4 to 2.1.5 (cosmos#656)

Bumps [actions/setup-go](https://github.com/actions/setup-go) from 2.1.4 to 2.1.5.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@v2.1.4...v2.1.5)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: colin axnér <[email protected]>

* docs: begin removal of internal "spec" directories (cosmos#634)

* begin removal of spec docs within core ibc

* remove broken link

* Apply suggestions from code review

Co-authored-by: Damian Nolan <[email protected]>

* remove broken link

* remove broken links

* Apply suggestions from code review

Co-authored-by: Carlos Rodriguez <[email protected]>

Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>

* Modify `OnChanOpenTry` application callback to perform app version negotitation (cosmos#646)

* remove NegotiateAppVersion and AppVersion gRPC (cosmos#643)

The NegotiateAppVersion callback has been removed from the IBC Application interface.
The gRPC AppVersion has been removed.
The app version negoitation will be handled by applications by returning the version in OnChanOpenTry.

* Modify `OnChanOpenTry` to return application version (cosmos#650)

* modify OnChanOpenTry to return negotiated version

modify IBCModule interface function OnChanOpenTry to return the negotiated app version. Tests have not been updated

* fix ibc_module_test.go tests

* fix tests

* Apply suggestions from code review

* add handshake test case

* add CHANGELOG and migration docs

* update documentation

* fix broken link

* fix broken link (cosmos#664)

* chore: update make build-docs, add docs build checker (cosmos#667)

* update Makefile, add docs build checker

* Update .github/workflows/check-docs.yml

Co-authored-by: Marko <[email protected]>

Co-authored-by: Marko <[email protected]>

* register ICA query server, fix panics in params query cli (cosmos#666)

Register the controller and host query servers to a chain.
Returns an error upon cli params query failure instead of panicing.

* update of roadmap with latest release (cosmos#653)

* update of roadmap with latest release and changed the way release versions are encoded

* fixed typo

Co-authored-by: Carlos Rodriguez <[email protected]>

* build(deps): bump actions/checkout from 2.3.1 to 2.4.0 (cosmos#672)

Bumps [actions/checkout](https://github.com/actions/checkout) from 2.3.1 to 2.4.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2.3.1...v2.4.0)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* allow ics20 to connect to middleware (cosmos#675)

* allow ics20 to connect to middleware

Creates ics4Wrapper which allows middleware applications to only implement SendPacket to connect ics20 as an application in its middleware stack

* add changelog and migration doc

* fix migration doc spelling

* fix: register InterchainAccount as x/auth GenesisAccount (cosmos#676)

* adding GenesisAccount interface registration for InterchainAccount impl

* updating RegisterInterfaces ica godoc

* enable mergify for backports (cosmos#678)

Co-authored-by: Carlos Rodriguez <[email protected]>

Co-authored-by: Sean King <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>

* fixes evmos tests

* cleanup

* changelog and cleanup

* pr suggestions. bump chainid

Co-authored-by: colin axnér <[email protected]>
Co-authored-by: Sean King <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Federico Kunze Küllmer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shaspitz shaspitz shaspitz approved these changes

@MSalopek MSalopek MSalopek approved these changes

@sainoe sainoe sainoe approved these changes

@jtremback jtremback Awaiting requested review from jtremback

@mpoke mpoke Awaiting requested review from mpoke

@mmulji-ic mmulji-ic Awaiting requested review from mmulji-ic

@glnro glnro Awaiting requested review from glnro

@yaruwangway yaruwangway Awaiting requested review from yaruwangway

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@danwt @sainoe @MSalopek @shaspitz