search

plume-sig / zk-nullifier-sig

Implementation of PLUME: nullifier friendly signature scheme on ECDSA
MIT License
128 stars 22 forks source link

Naming and its align #66

Closed skaunov closed 8 months ago

skaunov commented 8 months ago

See #64 for details. This additional PR created for merging conflict resolution.

skaunov commented 8 months ago

@Divide-By-0 I suggest merging commit without squashing -- could help for further debugging a bit if anything. Check and merge/close this one, pls.

Divide-By-0 commented 8 months ago

There shouldn't be both package lock and yarn lock :)

Divide-By-0 commented 8 months ago

Hey I'd love to merge but there are some conflicts, do you mind resolving those first?

skaunov commented 8 months ago

Actually I did. Twice. X)) To absorb #48 . So there was "22resolveConflict" branch (now deleted), then this one with trailing "", and it had the nice green button to choose between squash & merge commit. Then some of the following bump commits reintroduced the conflict. =((

Long story short: the problem I see here is that these lock files have substantial size and it's better to minimize the back and forth with them (last time I saw was via deprecated npm cache clean force instead of current verify). Just because we have monorepo which already have a mix of things, and lock-files cycles unproductively grow the repo. =(

I'm not that good in Git to touch the repo with force, though I see it as a most appropriate solution here. Another ways I see are following. 1) Revert bumping commits and merge PRs. But I believe it still will have that stuff in the revert commit. Or 2) I can do again what I've done while merging #48: delete both conflicting files and regenerate <javascript/yarn.lock>. I felt it's ok to do that once and live happily with them further; but as regular practice I feel it's bad. =(

Divide-By-0 commented 8 months ago

Yikes, good feedback. I'm open to any structural changes you'd recommend!

For one we should just gitignore package lock, overwriting yarn lock seems alright, I agree it's not an ideal practice. I don't really have any ideas here; maybe we can edit our dependabot settings somehow, or cut these lock files entirely and make versioning in package json more explicit?? Idk, I'm OK with whatever you choose.

skaunov commented 8 months ago

Yeah, sounds like a good idea to .gitignore the <javascript/package-lock.json> as a "dirty fix" to prevent reintroducing the problem by accident! Will do it.

So I currently stick to the path where I revert commits starting from https://github.com/plume-sig/zk-nullifier-sig/compare/f4b53bc1835aaccd114f9ab070b3cac381fabf04..main, and then merge this PR. This course let isolate the bloat in that one reverse commit so that someone fluent in Git could remove dead weight with force.

Will tackle this now or then.