search

filecoin-project / go-state-types

Primitive and low level types used in chain state and actor method parameters
Other
24 stars 37 forks source link

revert FIP-0070 related state change #231

Closed jennijuju closed 9 months ago

jennijuju commented 9 months ago

drop c656c180c4d2039bed342d678dca82428e45cfa4 #218

jennijuju commented 9 months ago

I'm confused by the diff here -- shouldn't this just be one commit, reverting FIP-0070 stuff? We can land it directly in master.

release/lotus-v1.24.0 was based off the latest go state type tag we used for nv21. master has new commits since we did the tags. I reverted the change by rebasing and drop the commit for fip0070 - I think that's where the diff is from

arajasek commented 9 months ago

I'm confused by the diff here -- shouldn't this just be one commit, reverting FIP-0070 stuff? We can land it directly in master.

release/lotus-v1.24.0 was based off the latest go state type tag we used for nv21. master has new commits since we did the tags. I reverted the change by rebasing and drop the commit for fip0070 - I think that's where the diff is from

Ah, I see. If we're taking this approach, we should just force-push to release/lotus-v1.24.0. I don't think we should do that, however because:

jennijuju commented 9 months ago

I'm confused by the diff here -- shouldn't this just be one commit, reverting FIP-0070 stuff? We can land it directly in master.

release/lotus-v1.24.0 was based off the latest go state type tag we used for nv21. master has new commits since we did the tags. I reverted the change by rebasing and drop the commit for fip0070 - I think that's where the diff is from

Ah, I see. If we're taking this approach, we should just force-push to release/lotus-v1.24.0. I don't think we should do that, however because:

  • having a clear revert commit is better IMO (part of chain history)
  • we do need the commit on master in our nv21 release (fixes broken invariants)

yah I don't have strong opinion here = made the change per your recommendation!