Skip to content
This repository was archived by the owner on Aug 14, 2024. It is now read-only.

Conversation

@fabnguess
Copy link
Contributor

@fabnguess fabnguess requested a review from fraxken March 16, 2023 16:59
@fabnguess
Copy link
Contributor Author

Hi @fraxken , I renamed the option to "isMemoize" due to potential conflicts. Note that I'm creating this draft to make sure I've got it right and that testing and documentation will follow.

@fraxken
Copy link
Member

fraxken commented Mar 16, 2023

@fabnguess no the argument must remain memoize. So we just have to found way to avoid collision (not degrade the code).

And please always open PR including documentation + test

@fabnguess fabnguess force-pushed the memoize_option_read branch from 8ece226 to 7439ee8 Compare March 16, 2023 18:28
@fabnguess fabnguess force-pushed the memoize_option_read branch 2 times, most recently from 27777a5 to 0a40739 Compare March 17, 2023 20:16
@fabnguess fabnguess force-pushed the memoize_option_read branch from 0a40739 to e1b331d Compare March 18, 2023 08:16
@fraxken fraxken changed the title Add a memoize option Add memoize option to read API Mar 18, 2023
@fraxken fraxken marked this pull request as ready for review March 18, 2023 13:06
@fraxken fraxken merged commit e663cec into main Mar 20, 2023
@fraxken fraxken deleted the memoize_option_read branch March 20, 2023 15:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add memoize option to read API

6 participants