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: x/mint docs and clean up, excluding mint keeper (backport #1857) #2072

Closed
wants to merge 1 commit into from

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jul 14, 2022

This is an automatic backport of pull request #1857 done by Mergify.
Cherry-pick of ee48cf5 has failed:

On branch mergify/bp/v10.x/pr-1857
Your branch is up to date with 'origin/v10.x'.

You are currently cherry-picking commit ee48cf58.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   Makefile
	modified:   docs/core/proto-docs.md
	modified:   proto/osmosis/mint/v1beta1/genesis.proto
	modified:   proto/osmosis/mint/v1beta1/mint.proto
	modified:   proto/osmosis/mint/v1beta1/query.proto
	renamed:    x/mint/spec/README.md -> x/mint/README.md
	modified:   x/mint/client/cli/cli_test.go
	modified:   x/mint/client/rest/grpc_query_test.go
	modified:   x/mint/genesis_test.go
	modified:   x/mint/keeper/hooks.go
	modified:   x/mint/module.go
	new file:   x/mint/simulation/export_test.go
	modified:   x/mint/simulation/genesis.go
	modified:   x/mint/simulation/genesis_test.go
	modified:   x/mint/types/events.go
	modified:   x/mint/types/expected_keepers.go
	new file:   x/mint/types/export_test.go
	modified:   x/mint/types/genesis.go
	modified:   x/mint/types/hooks.go
	modified:   x/mint/types/keys.go
	modified:   x/mint/types/mint.pb.go
	modified:   x/mint/types/minter.go
	modified:   x/mint/types/minter_test.go
	modified:   x/mint/types/query.pb.go

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   go.mod
	both modified:   proto/osmosis/superfluid/query.proto
	deleted by us:   x/mint/keeper/genesis.go
	both modified:   x/mint/keeper/hooks_test.go
	both modified:   x/mint/keeper/keeper.go
	both modified:   x/mint/types/genesis.pb.go
	both modified:   x/mint/types/params.go
	both modified:   x/pool-incentives/keeper/hooks.go
	both modified:   x/superfluid/types/query.pb.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

@p0mvn
Copy link
Member

p0mvn commented Jul 14, 2022

This generated a lot of unrelated pb.go files despite only changing mint protos. Could someone please take a look if everything is fine before I merge?

Seems that v10.x doesn't have protos up to date since running make proto-all from v10.x produced similar results

@ValarDragon
Copy link
Member

Is the reason for forcepushing due to resetting the branch to see the merge conflict?

Its pretty to hard to distinguish / double check backport changes vs everything else needed to get it mergeable with the forcepush

go.mod Outdated
@@ -15,6 +15,7 @@ require (
github.com/grpc-ecosystem/grpc-gateway v1.16.0
github.com/ory/dockertest/v3 v3.9.1
github.com/osmosis-labs/bech32-ibc v0.3.0-rc1
github.com/osmosis-labs/osmosis/v7 v7.3.0
Copy link
Member

@ValarDragon ValarDragon Jul 15, 2022

Choose a reason for hiding this comment

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

This backport fix was done wrong -- scanning for this was hard due to not separating out the backport vs the patch to make ti work!

We likely didn't update the import path for an introduced path by the change.

Copy link
Member

Choose a reason for hiding this comment

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

I fixed all import paths but this was left unused - thanks for catching that.

Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

See last comment about import

@ValarDragon ValarDragon requested a review from p0mvn July 15, 2022 02:14
@p0mvn p0mvn force-pushed the mergify/bp/v10.x/pr-1857 branch from 6feea69 to 0ab359e Compare July 15, 2022 02:19
@p0mvn
Copy link
Member

p0mvn commented Jul 15, 2022

Is the reason for forcepushing due to resetting the branch to see the merge conflict?

Its pretty to hard to distinguish / double check backport changes vs everything else needed to get it mergeable with the forcepush

Ah yeah, since there were many conflicts, I had to git reset HEAD~ fix them and then force push. When I have the files checked out, it is much more efficient to fix merge conflicts.

I will keep this in mind next time though and try to preserve history

@p0mvn p0mvn marked this pull request as draft July 15, 2022 02:26
@ValarDragon
Copy link
Member

Yeah that totally makes sense for fixing conflicts. TBH thats probably fine most of the time, this just had the unfortunate one-off of being mixed with the proto change

I backported @xBalbinus PR and fixed v10.x here: #2074

I suggest we merge that + get mergify to remake this backport (or we merge conflicts)

@p0mvn
Copy link
Member

p0mvn commented Jul 15, 2022

Will remake this after #2074

@p0mvn p0mvn closed this Jul 15, 2022
@mergify mergify bot deleted the mergify/bp/v10.x/pr-1857 branch July 15, 2022 02:45
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.

2 participants