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

[On-Chain] Ensure all message responses contain results #663

Closed
1 of 7 tasks
bryanchriswhite opened this issue Jul 5, 2024 · 4 comments · Fixed by #971, #972, #973, #975 or #974
Closed
1 of 7 tasks

[On-Chain] Ensure all message responses contain results #663

bryanchriswhite opened this issue Jul 5, 2024 · 4 comments · Fixed by #971, #972, #973, #975 or #974
Assignees
@bryanchriswhite
Labels
code health Cleans up some code community A ticket intended to potentially be picked up by a community member enhancement New feature or request
Milestone
Shannon Beta Test...

Comments

@bryanchriswhite
Copy link
Contributor

bryanchriswhite commented Jul 5, 2024 •
edited
Loading

Objective

Ensure message responses contain the respective created/updated resources.

Origin Document

image

Goals

  • Improve developer experience.

Deliverables

  • A PR for each module which updates all message reponse types to include the created/updated object.
  • A PR which adds a CLI tool which detects protobuf msg/query reponse types that don't contain any fields (proto and go source).
  • A PR which runs the CLI tool in CI to mitigate regressions on the main branch.

Non-goals / Non-deliverables

  • Refactoring protobuf types or message handlers outside the scope of this issue.

General deliverables

  • Comments: Add/update TODOs and comments alongside the source code so it is easier to follow.
  • Testing: Add new tests (unit and/or E2E) to the test suite.
  • Makefile: Add new targets to the Makefile to make the new functionality easier to use.
  • Documentation: Update architectural or development READMEs; use mermaid diagrams where appropriate.

Creator: @bryanchriswhite
Co-Owners: @red-0ne
@bryanchriswhite bryanchriswhite added enhancement New feature or request code health Cleans up some code community A ticket intended to potentially be picked up by a community member labels Jul 5, 2024
@bryanchriswhite

This comment was marked as resolved.

Sign in to view
@bryanchriswhite

This comment was marked as resolved.

Sign in to view
@bryanchriswhite bryanchriswhite moved this to 🔖 Ready in Shannon Jul 5, 2024
@bryanchriswhite bryanchriswhite self-assigned this Jul 5, 2024
@bryanchriswhite bryanchriswhite changed the title [REPLACE_WITH_IDENTIFIER] Provide a descriptive title [On-Chain] Ensure all message responses contain results Jul 5, 2024
@bryanchriswhite bryanchriswhite mentioned this issue Jul 5, 2024
Merged
14 tasks
@bryanchriswhite bryanchriswhite mentioned this issue Aug 21, 2024
Open
5 tasks
@bryanchriswhite
Copy link
Contributor Author

The issue outlined above should be resolved by #405 (comment)

@bryanchriswhite bryanchriswhite moved this from 🔖 Ready to 🏗 In progress in Shannon Nov 27, 2024
This was referenced Nov 28, 2024
Draft
Merged
Merged
Merged
Merged
Merged
@bryanchriswhite bryanchriswhite moved this from 🏗 In progress to 👀 In review in Shannon Dec 2, 2024
bryanchriswhite added a commit that referenced this issue Dec 9, 2024
@bryanchriswhite
[Code Health] Ensure supplier module msg responses are non-empty (#971)
af47606 
## Summary

Ensure all supplier module msg responses are non-empty. This adds a
`Supplier` field to the following protobuf type(s), and updates the unit
tests to assert for presence and correctness.

- `MsgStakeSupplierResponse`
- `MsgUnstakeSupplierResponse`

## Issue

- #663

## Type of change

Select one or more from the following:

- [ ] New feature, functionality or library
- [ ] Consensus breaking; add the `consensus-breaking` label if so. See
#791 for details
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Documentation
- [ ] Other (specify)

## Testing

- [ ] **Documentation**: `make docusaurus_start`; only needed if you
make doc changes
- [ ] **Unit Tests**: `make go_develop_and_test`
- [ ] **LocalNet E2E Tests**: `make test_e2e`
- [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.

## Sanity Checklist

- [x] I have tested my changes using the available tooling
- [ ] I have commented my code
- [x] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [ ] I have left TODOs throughout the codebase, if applicable
bryanchriswhite added a commit that referenced this issue Dec 9, 2024
@bryanchriswhite
[Code Health] Ensure gateway module msg responses are non-empty (#972)
a6e99b1 
## Summary

Ensure all supplier module msg responses are non-empty. This adds an
`Gateway` field to the following protobuf type(s), and updates the unit
tests to assert for presence and correctness.

- `MsgUnstakeGatewayResponse`

## Issue

- #663 

## Type of change

Select one or more from the following:

- [ ] New feature, functionality or library
- [ ] Consensus breaking; add the `consensus-breaking` label if so. See
#791 for details
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Documentation
- [ ] Other (specify)

## Testing

- [ ] **Documentation**: `make docusaurus_start`; only needed if you
make doc changes
- [ ] **Unit Tests**: `make go_develop_and_test`
- [ ] **LocalNet E2E Tests**: `make test_e2e`
- [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.

## Sanity Checklist

- [x] I have tested my changes using the available tooling
- [ ] I have commented my code
- [x] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [ ] I have left TODOs throughout the codebase, if applicable
bryanchriswhite added a commit that referenced this issue Dec 9, 2024
@bryanchriswhite
[Code Health] Ensure service module msg responses are non-empty (#973)
3faea85 
# Summary

Ensure all supplier module msg responses are non-empty. This adds a
`Service` field to the following protobuf type(s), and updates the unit
tests to assert for presence and correctness.

- `MsgAddServiceResopnse`

## Issue

- #663

## Type of change

Select one or more from the following:

- [ ] New feature, functionality or library
- [ ] Consensus breaking; add the `consensus-breaking` label if so. See
#791 for details
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Documentation
- [ ] Other (specify)

## Testing

- [ ] **Documentation**: `make docusaurus_start`; only needed if you
make doc changes
- [ ] **Unit Tests**: `make go_develop_and_test`
- [ ] **LocalNet E2E Tests**: `make test_e2e`
- [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.

## Sanity Checklist

- [ ] I have tested my changes using the available tooling
- [ ] I have commented my code
- [ ] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [ ] I have left TODOs throughout the codebase, if applicable
This was linked to pull requests Dec 9, 2024
[Tooling] Add protocheck responses subcmd to check for empty responses #970
Draft
[Code Health] Ensure supplier module msg responses are non-empty #971
Merged
[Code Health] Ensure gateway module msg responses are non-empty #972
Merged
[Code Health] Ensure service module msg responses are non-empty #973
Merged
bryanchriswhite added a commit that referenced this issue Dec 9, 2024
@bryanchriswhite
[Code Health] Ensure tokenomics module msg responses are non-empty (#975
0376986 
)

## Summary

Ensure all supplier module msg responses are non-empty. This adds a
`Params` field to the following protobuf type(s), and updates the unit
tests to assert for presence and correctness.

- `MsgUpdateParamsResponse`

## Issue

- #663

## Type of change

Select one or more from the following:

- [ ] New feature, functionality or library
- [ ] Consensus breaking; add the `consensus-breaking` label if so. See
#791 for details
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Documentation
- [ ] Other (specify)

## Testing

- [ ] **Documentation**: `make docusaurus_start`; only needed if you
make doc changes
- [ ] **Unit Tests**: `make go_develop_and_test`
- [ ] **LocalNet E2E Tests**: `make test_e2e`
- [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.

## Sanity Checklist

- [x] I have tested my changes using the available tooling
- [ ] I have commented my code
- [x] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [ ] I have left TODOs throughout the codebase, if applicable
This was linked to pull requests Dec 9, 2024
[Code Health] Ensure tokenomics module msg responses are non-empty #975
Merged
[Code Health] Ensure application module msg responses are non-empty #974
Merged
bryanchriswhite added a commit that referenced this issue Dec 9, 2024
@bryanchriswhite
[Code Health] Ensure application module msg responses are non-empty (#…
23b4c88 
…974)

## Summary

Ensure all application module msg responses are non-empty. This adds a
`Application` field to the following protobuf type(s), and updates the
unit tests to assert for presence and correctness.

- `MsgUnstakeApplicationResponse`
- `MsgUndelegateFromGatewayResponse`

## Issue

- #663

## Type of change

Select one or more from the following:

- [ ] New feature, functionality or library
- [ ] Consensus breaking; add the `consensus-breaking` label if so. See
#791 for details
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Documentation
- [ ] Other (specify)

## Testing

- [ ] **Documentation**: `make docusaurus_start`; only needed if you
make doc changes
- [ ] **Unit Tests**: `make go_develop_and_test`
- [ ] **LocalNet E2E Tests**: `make test_e2e`
- [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.

## Sanity Checklist

- [x] I have tested my changes using the available tooling
- [ ] I have commented my code
- [x] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [ ] I have left TODOs throughout the codebase, if applicable
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Shannon Dec 9, 2024
@bryanchriswhite
Copy link
Contributor Author

Closing as the fixes have all been merged. NOTE: #970 is still in progress and is what automates the mitigation of these issues going forward, once integrated into CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bryanchriswhite bryanchriswhite

Labels
code health Cleans up some code community A ticket intended to potentially be picked up by a community member enhancement New feature or request
Projects
Shannon
Status: ✅ Done
Milestone
Shannon Beta TestNet Support
2 participants
@bryanchriswhite @Olshansk