Skip to content

Rename to rollmint#545

Merged
tzdybal merged 13 commits intomainfrom
tzdybal/rollmint
Oct 4, 2022
Merged

Rename to rollmint#545
tzdybal merged 13 commits intomainfrom
tzdybal/rollmint

Conversation

@tzdybal
Copy link
Copy Markdown
Contributor

@tzdybal tzdybal commented Sep 29, 2022

  • rename go module
  • rename imports
  • rename all comments
  • rename variables/structs/etc with opti and optimint in their name
  • fix links (badges/etc)
  • update ADRs
  • grep entire directory to ensure full rename

It's a part of: #535

@tzdybal tzdybal self-assigned this Sep 29, 2022
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #545 (857dd06) into main (722b44e) will increase coverage by 0.04%.
The diff coverage is 39.42%.

❗ Current head 857dd06 differs from pull request most recent head 52cafd5. Consider uploading reports for the commit 52cafd5 to get more accurate results

@@            Coverage Diff             @@
##             main     #545      +/-   ##
==========================================
+ Coverage   55.76%   55.80%   +0.04%     
==========================================
  Files          50       50              
  Lines        9544     9544              
==========================================
+ Hits         5322     5326       +4     
+ Misses       3428     3425       -3     
+ Partials      794      793       -1     
Impacted Files Coverage Δ
config/config.go 86.95% <ø> (ø)
conv/abci/block.go 94.23% <ø> (ø)
conv/addr.go 100.00% <ø> (ø)
conv/config.go 47.05% <ø> (ø)
da/celestia/celestia.go 50.00% <ø> (ø)
da/celestia/mock/server.go 50.32% <ø> (ø)
da/grpc/grpc.go 73.13% <ø> (ø)
da/mock/mock.go 79.54% <ø> (ø)
da/registry/registry.go 100.00% <ø> (ø)
mempool/v1/mempool.go 86.09% <ø> (ø)
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tzdybal tzdybal marked this pull request as ready for review September 29, 2022 11:38
@tzdybal
Copy link
Copy Markdown
Contributor Author

tzdybal commented Sep 29, 2022

@jeremysklaroff added you because of: 2de7d62

Copy link
Copy Markdown
Contributor

@MSevey MSevey left a comment

Choose a reason for hiding this comment

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

All my comments are nits really and could be handled in a small follow up if necessary.

mv ethermint ..
cd ..
cp -R optimint ethermint
cp -R rollmint ethermint
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: we are renaming it once which means we might rename it again. Or if someone forks this and wants to name it differently they are going to need to find and replace again.

So could we just use a env variable up top? Like DIR_NAME?

You could then use that var in a number of places.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Created #546,

Comment thread proto/get_deps.sh Outdated
cd "$(dirname "${BASH_SOURCE[0]}")"

TM_VERSION=v0.34.14
TM_VERSION=v0.34.21
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sneaky

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My bad. Reverted this change and created: #547

Comment thread proto/tendermint/abci/types.proto Outdated
repeated Event events = 7
[(gogoproto.nullable) = false, (gogoproto.jsontag) = "events,omitempty"];
string codespace = 8;
string sender = 9;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't feel like a rename :-)

Is this bringing in some other changes to make the merge easier?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the result of updating TM to 0.34.21. I'll create separate PR for this; I missed this while working on #528.

repository: celestiaorg/ethermint
path: ethermint
ref: optimint-v0.3.0-rebase
ref: rollmint-v0.3.0-rebase
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Can't you use TAG_PREFIX here?

repository: celestiaorg/ethermint
path: ethermint
ref: optimint-v0.3.0-rebase
ref: rollmint-v0.3.0-rebase
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same TAG_PREFIX

- name: "Setup Cluster"
run: |
export ETHERMINT_IMAGE_TAG=optimint-$(git rev-parse --short "$GITHUB_SHA")
export ETHERMINT_IMAGE_TAG=rollmint-$(git rev-parse --short "$GITHUB_SHA")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same TAG_PREFIX

@tzdybal tzdybal requested review from MSevey and musalbas October 3, 2022 11:10
Copy link
Copy Markdown
Contributor

@MSevey MSevey left a comment

Choose a reason for hiding this comment

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

Docker Github Actions are failing but otherwise looks good

@tzdybal tzdybal requested a review from MSevey October 3, 2022 15:12
@tzdybal
Copy link
Copy Markdown
Contributor Author

tzdybal commented Oct 3, 2022

Those jobs are expected to fail, sorry for not mentioning this. I need to update cosmos-sdk to use proper version of rollmint, to "fix" those builds. (But I need "rollmint" in the first place)

@tzdybal tzdybal merged commit cc0a295 into main Oct 4, 2022
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.

3 participants