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

Use @inlinable and remove redundant helper function #105

Merged
merged 7 commits into from
Jan 11, 2023

Conversation

nkristek
Copy link
Collaborator

@nkristek nkristek commented Nov 24, 2022

This is a tiny PR that changes two things:

  • Use Dictionary(grouping:by:) instead of the custom implementation that was used previously
  • Set @inlinable instead of @usableFromInline on internal helper functions, that are small enough to benefit from inlining. This allows for better compiler optimisations, since the content of the inlined helper function can be inlined itself, instead of it just being able to be called from within an inlined function.
  • Improve description of Error.moveIsNotInTheSameSection and Error.adapterDoesNotContainSectionController
  • Fix snapshot tests in example project

Niclas Kristek added 3 commits November 24, 2022 16:57
Signed-off-by: Niclas Kristek <dev@nkristek.com>
This allows for better compiler optimisations, since the content of the inlined helper function can be inlined itself, instead of it just being able to be called from within an inlined function.

Signed-off-by: Niclas Kristek <dev@nkristek.com>
Signed-off-by: Niclas Kristek <dev@nkristek.com>
@Marcocanc
Copy link
Member

@nkristek somehow the Unit Tests aren't running, potentially due to the force push. I also don't have the option to re-run on my end. Could you push something to retrigger the checks?

Niclas Kristek added 2 commits January 10, 2023 16:44
Signed-off-by: Niclas Kristek <dev@nkristek.com>
Signed-off-by: Niclas Kristek <dev@nkristek.com>
@nkristek
Copy link
Collaborator Author

@Marcocanc could you please retry running the workflow? I've pushed two small commits that improve the description of some error cases.

Signed-off-by: Niclas Kristek <dev@nkristek.com>
@nkristek
Copy link
Collaborator Author

I've just noticed, that in the example project the snapshot tests were broken. They're fixed as well now.

Signed-off-by: Niclas Kristek <dev@nkristek.com>
@Marcocanc
Copy link
Member

Running now. I also changed the settings so you don't need approval for every run.

@nkristek nkristek merged commit 91c5615 into traderepublic:develop Jan 11, 2023
@nkristek nkristek deleted the inlinable_and_simplify_code branch January 11, 2023 16:12
@nkristek nkristek mentioned this pull request Jan 13, 2023
nkristek pushed a commit that referenced this pull request Jan 13, 2023
The recent changes may be source breaking, thus the next release needs
to be a new major version.

Changelog:
- Add `MainActor` & `Sendable` annotation to relevant types (#101)
- Fix snapshot testing dependency in example project (#102)
- Fix Xcode 14 compiler warnings (#103)
- Use `@inlinable` and remove redundant helper function (#105)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants