Skip to content

Conversation

@0xTim
Copy link
Member

@0xTim 0xTim commented Jun 29, 2022

Adds the ability to control starting, committing and rolling back transactions outside of the main Fluent API. This could be used for setting up tests or when you need more control over a transaction.

Introduces a new TransactionControlDatabase protocol to implement, that conforms to Database

Warning: It is the users' responsibility to ensure the handle errors and rollback when necessary and commit transactions

@codecov-commenter
Copy link

codecov-commenter commented Jun 29, 2022

Codecov Report

Merging #520 (7c553db) into main (2b3ac05) will increase coverage by 3.21%.
The diff coverage is n/a.

❗ Current head 7c553db differs from pull request most recent head f8ddd14. Consider uploading reports for the commit f8ddd14 to get more accurate results

@@            Coverage Diff             @@
##             main     #520      +/-   ##
==========================================
+ Coverage   43.48%   46.70%   +3.21%     
==========================================
  Files         109       98      -11     
  Lines        6057     5548     -509     
==========================================
- Hits         2634     2591      -43     
+ Misses       3423     2957     -466     
Flag Coverage Δ
unittests 46.70% <ø> (+3.21%) ⬆️

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

Impacted Files Coverage Δ
Sources/FluentBenchmark/SolarSystem/Governor.swift
Sources/FluentBenchmark/SolarSystem/Tag.swift
...ources/FluentBenchmark/SolarSystem/PlanetTag.swift
...ntBenchmark/SolarSystem/GalacticJurisdiction.swift
Sources/FluentBenchmark/SolarSystem/Planet.swift
Sources/FluentBenchmark/SolarSystem/Star.swift
Sources/FluentBenchmark/SolarSystem/Galaxy.swift
Sources/FluentBenchmark/SolarSystem/Moon.swift
Sources/FluentBenchmark/FluentBenchmarker.swift
...ces/FluentBenchmark/SolarSystem/Jurisdiction.swift
... and 3 more

Copy link
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

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

LGTM, but can we add a documentation comment on the protocol itself warning about the caveats of using it?

@0xTim
Copy link
Member Author

0xTim commented Jun 30, 2022

Yes adding docs would actually be useful 🤦

@0xTim 0xTim requested a review from gwynne June 30, 2022 14:32
Copy link
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

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

Couple of grammar and style nits, but otherwise LGTM

Co-authored-by: Gwynne Raskind <gwynne@vapor.codes>
@0xTim 0xTim merged commit 2676424 into main Jun 30, 2022
@0xTim 0xTim deleted the transaction-additions branch June 30, 2022 14:46
@VaporBot
Copy link

These changes are now available in 1.31.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-minor Contains new APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants