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

chore(rollapp): added x/rollapp proto files #1079

Merged
merged 4 commits into from
Sep 19, 2024
Merged

Conversation

keruch
Copy link
Contributor

@keruch keruch commented Sep 18, 2024

PR Standards

Close #1078

Dymint imported the Hub previously, but now Cosmos versions are different as well as other deps, so direct import doesn't work currently. As a workaround, the autogenerated proto contracts were copied from the Hub to Dymint. Copying autogenerated files is not safe (it can lead to dependencies mismatch), it's impossible to find what was the original proto for these files, and it's extremely inobvious how to add new fields to such proto contracts (apparently find a suitable generator, generate files using it, and then manually copy them?).

This PR moves x/rollapp proto contracts from the Hub to Dymint, so the files are generated directly in the Dymint repo with its own proto generator.


For Author:

  • Targeted PR against correct branch
  • included the correct type prefix in the PR title
  • Linked to Github issue with discussion and accepted design
  • Targets only one github issue
  • Wrote unit and integration tests
  • All CI checks have passed
  • Added relevant godoc comments

For Reviewer:

  • confirmed the correct type prefix in the PR title
  • Reviewers assigned
  • confirmed all author checklist items have been addressed

After reviewer approval:

  • In case targets main branch, PR should be squashed and merged.
  • In case PR targets a release branch, PR should be rebased.

@keruch keruch self-assigned this Sep 18, 2024
@keruch keruch requested a review from a team as a code owner September 18, 2024 08:18
@mtsitrin
Copy link
Contributor

@keruch awesome!

can u do the same for the x/sequencer and get rid of the third_party/dymension/sequencer copied files?

@mtsitrin mtsitrin merged commit 4793c07 into main Sep 19, 2024
6 checks passed
@mtsitrin mtsitrin deleted the kirill/rollapp-proto branch September 19, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not use copied autogenerated files for x/rollapp
2 participants