Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve strategy names #1053

Closed
jams2 opened this issue Nov 22, 2023 · 1 comment · Fixed by #1058
Closed

Improve strategy names #1053

jams2 opened this issue Nov 22, 2023 · 1 comment · Fixed by #1058

Comments

@jams2
Copy link

jams2 commented Nov 22, 2023

The problem

The strategy names build and create are not distinct enough in meaning from each other. This means it's hard to remember which does what. In an informal slack poll at my workplace, ~10% of developers agreed with me that they needed to look up the documentation frequently to be sure they were correct. I have worked extensively on the project https://github.com/wagtail/wagtail-factories, so consider myself reasonably familiar with factory_boy, but struggle to remember which strategy is which!

Proposed solution

I would love to see aliases to the .build and .create methods in factory_boy that more explicitly communicate what they do. Off the top of my head, .in_memory and .persist seem like reasonable options, but I'm open to bikeshedding.

I am happy to contribute a PR to this effect if this is agreeable.

@Viicos
Copy link
Contributor

Viicos commented Jan 17, 2024

I don't think creating aliases would be the way to go; in my experience, it doesn't add any benefit and confuse people even more. Users discovering the library will want to know the different between build and in_memory (or create and persist) just to discover they mean the same thing.

However, I think the docstrings could be improved. If you are using an IDE with a LSP integration, you'll be able to know which method is doing what:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants