cronos-labs / cronos-zkevm

Apache License 2.0
8 stars 2 forks source link

Problem: sync upstream repo to v18.13.0 #42

Closed JayT106 closed 4 months ago

JayT106 commented 8 months ago

What ❔

sync upstream repo to v18.13.0

thomas-nguy commented 8 months ago

thanks for the PR

I think it is just easier at the moment to rebase our changes on top of their release branch

reason is that our changes are quite minimal so it is more manageable to fix our changes rather than fixing conflicts on hundreds of commits

JayT106 commented 8 months ago

thanks for the PR

I think it is just easier at the moment to rebase our changes on top of their release branch

  • checkout 18.13.0 release branch
  • cherry pick last commit from main branch
  • review and force push to main branch

reason is that our changes are quite minimal so it is more manageable to fix our changes rather than fixing conflicts on hundreds of commits

So current main branch doesn't include our cronos changes. Do you want me to cherry-pick the cronos changes and do force push directly?

Personally I would like to do PR so we can keep the code changes history. It will be more easily to maintain the repo (if we need rollback the code in future). But it requires doing Rebase and merge settings in PR

thomas-nguy commented 8 months ago

Yeah I understand but our custom changes is just related to base token implementation at the moment

Once this get merged, we can completely switch to upstream solution and discard our changes

https://github.com/matter-labs/era-contracts/pull/29

Also once modularisation in the zkstack is completed, we will be able to have our own customisation and commits without having to fix conflict when rebasing with upstream everytime.

For the time being, cherry picking seems the easiest solution

I think we should also help them to modularize the repo. Maybe moving the scripts to a separate location could help or modularizaing the scripts to allow custom changes but this need to be discussed upstream