solady icon indicating copy to clipboard operation
solady copied to clipboard
verified publisher icon Vectorized

♻️ Simplify PRNG import by defining struct externally

Open Philogy opened this issue 1 year ago • 4 comments

Description

I moved the PRNG and LazyShuffler structs outside of library LibPRNG so that they could be individually imported and you didn't always have to type LibPRNG.PRNG.

Note: This is a breaking change.

Checklist

Ensure you completed all of the steps below before submitting your pull request:

  • [x] Ran forge fmt?
  • [x] Ran forge snapshot?
  • [x] Ran forge test?

Philogy avatar Jul 02 '24 14:07 Philogy

😱 I’m allergic to the inconsistency. 😱

Vectorized avatar Jul 02 '24 14:07 Vectorized

😱 I’m allergic to the inconsistency. 😱

What inconsistency? Is this not done for the other libs, I can open larger PR to move it all. Although CI is failing me because it's not compiling for older versions, I thought top-level using statements were introduced in 0.8.4

Philogy avatar Jul 02 '24 15:07 Philogy

I prefer the types to be in the library itself.
The verbose ritual of typing LibPRNG.PRNG reminds me that this struct belongs to the LibPRNG namespace.
It gives me dopamine.

If we make it consistent across all files, LibRLP.List memory list will become RLPList memory list. Which doesn’t feel good to me.

Vectorized avatar Jul 03 '24 03:07 Vectorized

Also, this has been on the wish list for years. Not sure why the team deprioritised this small syntax sugar QoL thingy over generics.

https://github.com/ethereum/solidity/issues/1100

Vectorized avatar Jul 03 '24 04:07 Vectorized