rename peggy to gravity #278
Conversation
…ridge into marko/rename_gravity
|
will clean this up |
tac0turtle
requested review from
jackzampolin,
jkilpatr and
mvid
as code owners
|
This pull request introduces 1 alert and fixes 1 when merging b58110e into fd98d20 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request fixes 1 alert when merging 2f403b7 into fd98d20 - view on LGTM.com fixed alerts:
|
|
I will be reverting this change it doesn't work at all. All the tests are failing, etc |
|
which part doesnt work? We can spend sometime fixing the tests instead of reverting. |
|
Message scopes need to be fixed in the orchestrator (gravity/MsgWhatever versus peggy/MsgWhatever), I don't think there's too much more to do. But we have no idea if there is because the tests are just obviously broken, you can't just merge a patch this big and not even look at the tests. We have no idea if this works all up and 'merge it and make it work after' is not an acceptable coding practice. |
I'll set some ci restrictions to avoid this in the future. No need to revert. No need to bring up bad coding practice. This repo is not setup correctly, and we are all trying our best to instill best practices |
I can't find this any messages with peggy/Msg*. These were changed. |
The tests still have some flake issues (although those have been greatly reduce recently) and the darn arbitrary logic test needs a repo secret that's not avaialble to outside PRs (so it will always fail if someone external opens a PR). My opinion on CI design after being a CI engineer for a couple of years is that it must always be up to engineers to look at the output and understand it and the managers to hold their reports responsible for not doing so. Trying to run greenlight / redlight CI systems was just a mess at scale.
I see that now. With the obvious out of the way that means there's some more subtle bug. Oh the rust code was renamed but the folders where not. Lets see where fixing that gets us. |
|
working on that now 👍 Noticed that after trying to run cargo build a few times 😆 |
|
working on it here https://github.com/cosmos/gravity-bridge/pull/306/files |
|
I think I've got it. There's one trick to it. The proto regen, which should be in a readme somewhere. |
|
there was also no renaming in the solidity directory. so everything over there needs to be done. Looks like just renaming Peggy.json to Gravity.json works |
|
There we go looks like this is resolved. I'll rebase the rest of the PRs this morning. You did a good job with this pr @marbar3778 it just needed a little more attention to get everything working. Just pm'ing me that you wanted this merged would have been enough to get my attention and this fixed up. If we had been unlucky and some unexpected problem was waiting in the wings this could have borked main for days or weeks. Putting us in a bad spot between reverting and losing work and having anything that works at all. Considering we are all actively running testnets this isn't acceptable. I know things are running behind schedule here and it does take me time to get to PRs and help explain how some of the other Gravity components work. But we're all professionals here, I don't think we need to hide the merge button like we would from a child. It's reasonable to expect everyone to be conscientious and capable. |
Sorry about that. Tried to set up the bot to request the needed people, may not have worked correctly. Will ping you in tg next time 😄 |
No description provided.