search

Montspy / LooPyGen

Loopring Python Minter on Layer 2
17 stars 6 forks source link

Add --overwrite flag to rewrite every metadata json files #28

Closed Montspy closed 2 years ago

Montspy commented 2 years ago

New --preserve flag for metadata.py. It will only update CIDs in the metadata.json files that exist. If the json is not valid, it will print out an error, save a backup of the file (add .bak suffix) and regenerate a new metadata json file

Montspy commented 2 years ago

This implements #19

Montspy commented 2 years ago

@sk33z3r Should we make the default behavior to preserve metadata and have an --overwrite flag instead?

sk33z3r commented 2 years ago

Should we make the default behavior to preserve metadata and have an --overwrite flag instead?

Seems to feel more "user friendly" if you automatically overwrite so nothing is ever stale. I can't come up with an argument where you would want to keep the CID list stale like that, but I'm sure there is a use case I'm not thinking of.

At the end of the day, I make a change to a file and try to mint, I don't expect to have to regen CID because it's sort of a hidden feature to the user right now. Might not be completely obvious that the CID calc was a step in the process.

Montspy commented 2 years ago

Agreed, in all cases, running metadata should always "refresh" at least the CIDs. a) Current behavior is to overwrite the whole metadata file. Including any manual edits. b) My old proposal (--preserve) was to keep this as default behavior. The --preserve flag would only update the CIDs nad keep any metadata edits. c) My new proposal (--overwrite) is to change the default behavior to refresh only the CIDs and --overwrite would overwrite the whole metadata file.

I believe behavior c) to be less prone to data loss for users who manually edit their metadata after generation, while preventing stale CIDs for all users

Montspy commented 2 years ago

Changed the default behavior to only update CIDs Added --overwrite flag to force rewriting new metadata json files

sk33z3r commented 2 years ago

I believe behavior c) to be less prone to data loss for users who manually edit their metadata after generation, while preventing stale CIDs for all users

Yea, I agree with that and a good move to make it default with this option.