search

RafaelAPB / blockchain-integration-framework

A new approach to the blockchain interoperability problem
Apache License 2.0
6 stars 3 forks source link

feat(SATP-Hermes): gateway coordinator #73

Closed AndreAugusto11 closed 7 months ago

petermetz commented 7 months ago

@AndreAugusto11 Also another note: The diff on this pull request is already HUGE so I highly recommend thinking about what smaller (and yet self-contained) changes are embedded in this huge diff that you could extract/make into a smaller (easier to review) pull request as of today. It would be better for everyone if we got those smaller ones going and then you just kept rebasing this branch onto main as we are merging in the smaller pull requests piece by piece. Otherwise we'll probably have a hard time reviewing this and you'll have ha hard time addressing change requests because there'll be so many of them.

AndreAugusto11 commented 7 months ago

@petermetz I've added a commit fixing those details you pointed out. I'm having some issues resolving the conflicts with dev. The dev is rebased with the latest changes from the upstream (cacti main repo). Could you rebase and trigger the CI again pls? Once that's done will retest again on my machine and see what's going on my local setup

AndreAugusto11 commented 7 months ago

@AndreAugusto11 Also another note: The diff on this pull request is already HUGE so I highly recommend thinking about what smaller (and yet self-contained) changes are embedded in this huge diff that you could extract/make into a smaller (easier to review) pull request as of today. It would be better for everyone if we got those smaller ones going and then you just kept rebasing this branch onto main as we are merging in the smaller pull requests piece by piece. Otherwise we'll probably have a hard time reviewing this and you'll have ha hard time addressing change requests because there'll be so many of them.

Thank you for your input! Rafael and I are preparing the base so that other people (on our side) can start working on their tasks.

Once this is done, we will most likely separate all these changes into some PRs to ease everyone's job.

petermetz commented 7 months ago

@petermetz I've added a commit fixing those details you pointed out. I'm having some issues resolving the conflicts with dev. The dev is rebased with the latest changes from the upstream (cacti main repo). Could you rebase and trigger the CI again pls? Once that's done will retest again on my machine and see what's going on my local setup

@AndreAugusto11 I squashed all the commits together otherwise it would've taken 10x the time to rebase, I hope that's OK. The other trick I used is to reset the yarn.lock to the upstream/main branch's state before I begin to rebase: git checkout upstream/main -- yarn.lock that way when I was doing the rebase the yarn.lock file weird issues (that I also had no idea how to solve) did not even come up.

AndreAugusto11 commented 7 months ago

@petermetz I've added a commit fixing those details you pointed out. I'm having some issues resolving the conflicts with dev. The dev is rebased with the latest changes from the upstream (cacti main repo). Could you rebase and trigger the CI again pls? Once that's done will retest again on my machine and see what's going on my local setup

@AndreAugusto11 I squashed all the commits together otherwise it would've taken 10x the time to rebase, I hope that's OK. The other trick I used is to reset the yarn.lock to the upstream/main branch's state before I begin to rebase: git checkout upstream/main -- yarn.lock that way when I was doing the rebase the yarn.lock file weird issues (that I also had no idea how to solve) did not even come up.

No problem! Thanks! That was the issue I was having and did not know how to solve it otherwise

AndreAugusto11 commented 7 months ago

@petermetz @RafaelAPB it seems the build-dev is good to go 😀. I'm going to fix the rest of the things missing in the CI

petermetz commented 7 months ago

@petermetz @RafaelAPB it seems the build-dev is good to go 😀. I'm going to fix the rest of the things missing in the CI

@AndreAugusto11 Yay! 😊 If you need help with breaking it down to smaller, self contained commits (now or in the future) just let me know!

AndreAugusto11 commented 7 months ago

@petermetz @RafaelAPB it seems the build-dev is good to go 😀. I'm going to fix the rest of the things missing in the CI

@AndreAugusto11 Yay! 😊 If you need help with breaking it down to smaller, self contained commits (now or in the future) just let me know!

TY very much for the helpl!!