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

Bugfix/nullable transaction return #559

Merged

Conversation

mqus
Copy link
Collaborator

@mqus mqus commented May 1, 2021

Fixes #557 and maybe adresses #558

@codecov
Copy link

codecov bot commented May 1, 2021

Codecov Report

Merging #559 (edda3a8) into develop (1c14faf) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #559      +/-   ##
===========================================
- Coverage    90.08%   90.06%   -0.02%     
===========================================
  Files           74       74              
  Lines         1855     1852       -3     
===========================================
- Hits          1671     1668       -3     
  Misses         184      184              
Flag Coverage Δ
floor 90.42% <ø> (ø)
floor_generator 90.02% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...enerator/lib/writer/transaction_method_writer.dart 100.00% <ø> (ø)
...r_generator/lib/processor/queryable_processor.dart 96.15% <100.00%> (-0.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c14faf...edda3a8. Read the comment docs.

@mqus mqus force-pushed the bugfix/nullable-transaction-return branch from 84b8ab2 to 8d49080 Compare May 1, 2021 12:56
Copy link
Collaborator

@vitusortner vitusortner left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix!

@@ -14,7 +14,7 @@ class TransactionMethodWriter implements Writer {
return Method((builder) => builder
..annotations.add(overrideAnnotationExpression)
..returns = refer(method.returnType.getDisplayString(
withNullability: false,
withNullability: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

❓ Do we also get support for the case described in #557 where we don't return a nullable Future but a Future wrapping a nullable type (e.g Future<Task?>)? If so, could you add another test to verify the behavior?

Copy link
Collaborator Author

@mqus mqus May 2, 2021

Choose a reason for hiding this comment

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

I think we had that support already, but I'll add a test.

@vitusortner vitusortner added the fix Implementation of a bug fix (release drafter) label May 2, 2021
@vitusortner vitusortner merged commit 57b4899 into pinchbv:develop May 2, 2021
@mqus mqus deleted the bugfix/nullable-transaction-return branch March 12, 2022 19:09
@Nath4lie
Copy link

Nath4lie commented Jun 2, 2022

I've tried to return Future<Object?> from transaction but it still causes an error in generated db file. Will there be support for return Future with nullable object?

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Implementation of a bug fix (release drafter)
Development

Successfully merging this pull request may close these issues.

Transaction: a nullable return type becomes not nullable in the method's implementation
3 participants