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.

Sign up for GitHub

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

Jump to bottom

Refactor base factory.get_field_value #269

Merged
merged 3 commits into from
Jun 28, 2023
Merged

Refactor base factory.get_field_value #269

merged 3 commits into from
Jun 28, 2023

Conversation

gsakkis
Copy link
Contributor

@gsakkis gsakkis commented Jun 27, 2023

Pull Request Checklist

  • New code has 100% test coverage
  • (If applicable) The prose documentation has been updated to reflect the changes introduced by this PR
  • (If applicable) The reference documentation has been updated to reflect the changes introduced by this PR

Description

Returning a field value from a factory is currently handled by one of four different callables:

  1. BaseFactory.get_field_value
  2. BaseFactory.get_mock_value
  3. complex_types.handle_complex_type
  4. complex_types.handle_collection_type

Even worse, these callables may call each other recursively. The end result is complexity, bugs such as #239 and code duplication to fix such bugs.

This PR merges (2) and (3) into (1), leaving just two callables in place of four.

@gsakkis gsakkis requested a review from a team as a code owner June 27, 2023 18:13
@sourcery-ai sourcery-ai bot mentioned this pull request Jun 27, 2023
Closed
@gsakkis gsakkis changed the title Refactor base factory.get field value Refactor base factory.get_field_value Jun 27, 2023
@sourcery-ai sourcery-ai bot mentioned this pull request Jun 28, 2023
Closed
JacobCoffee
JacobCoffee approved these changes Jun 28, 2023
View reviewed changes
Copy link
Member

@JacobCoffee JacobCoffee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing thing is impl. in #281

@Goldziher Goldziher merged commit f5c247f into main Jun 28, 2023
@Goldziher Goldziher deleted the refactor-BaseFactory.get_field_value branch June 28, 2023 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JacobCoffee JacobCoffee JacobCoffee approved these changes

@Goldziher Goldziher Goldziher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gsakkis @Goldziher @JacobCoffee