Skip to content

refactor(core): break manager struct into smaller ones#8124

Merged
lucasfernog merged 12 commits intodevfrom
refactor/manager
Nov 7, 2023
Merged

refactor(core): break manager struct into smaller ones#8124
lucasfernog merged 12 commits intodevfrom
refactor/manager

Conversation

@lucasfernog
Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

@lucasfernog lucasfernog requested a review from a team as a code owner October 27, 2023 13:42
@lucasfernog lucasfernog changed the title refactor(core): break manager struct refactor(core): break manager struct into smaller ones Oct 27, 2023
Copy link
Copy Markdown
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

I think my main problem with this PR is the excessive usage of Arc, do you think we can somehow remove the need for it?

Edit: the Arc usage was excessive before as well, I guess I misspoke, what I mean, can we reduce the Arc usage now that we have grouped multiple fields together?

@lucasfernog
Copy link
Copy Markdown
Member Author

@amrbashir I've done my best with the Arc removals, some are still around because there's some types that are cloned and moved to another thread / closure (mostly callbacks).

@lucasfernog lucasfernog merged commit 04a682b into dev Nov 7, 2023
@lucasfernog lucasfernog deleted the refactor/manager branch November 7, 2023 12:58
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 this pull request may close these issues.

2 participants