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

Various CE3 improvements #347

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

dangerousben
Copy link
Contributor

Main changes:

  • Use the SyncGenerators provided by the CE3 testkit to generate interesting Rerunnable instances
  • Catbird-native unsafeRun (just using Await for now)
  • Use the twitter-util provided Time and Stopwatch for the Clock methods

Divided up into small commits so should be easy enough to pull out anything that's contentious.

@felixbr this could go in before or after #267, I'm not sure what that's currently waiting on?

@felixbr
Copy link
Contributor

felixbr commented Jun 14, 2021

I've merged my branch, so you can base your PR(s) on master without being blocked by me.

@dangerousben dangerousben force-pushed the feature/ce3-improvements branch from 02db891 to bea885c Compare June 14, 2021 18:08
@dangerousben dangerousben changed the base branch from topic/cats-effect-3.0.0 to master June 14, 2021 18:08
@dangerousben dangerousben force-pushed the feature/ce3-improvements branch from bea885c to 57b83f3 Compare June 15, 2021 07:36
@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2021

Codecov Report

Merging #347 (57b83f3) into master (4c92e11) will decrease coverage by 1.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
- Coverage   91.12%   90.10%   -1.03%     
==========================================
  Files          17       17              
  Lines         293      293              
  Branches        1        1              
==========================================
- Hits          267      264       -3     
- Misses         26       29       +3     
Impacted Files Coverage Δ
...a/io/catbird/util/effect/RerunnableInstances.scala 100.00% <100.00%> (ø)
...rc/main/scala/io/catbird/util/effect/package.scala 9.09% <0.00%> (-45.46%) ⬇️
...il/src/main/scala/io/catbird/util/Rerunnable.scala 92.68% <0.00%> (+2.43%) ⬆️

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 4c92e11...57b83f3. Read the comment docs.

@DuncanHills
Copy link

Hi @dangerousben do you need any help getting this merged? I have a catbird-dependent project that I'd like to upgrade to CE3 soon and I'm taking an interest in improving this project.

@felixbr
Copy link
Contributor

felixbr commented Nov 11, 2021

@DuncanHills I don't have time to review this but if you could do a quick review I'd be happy to merge this in. I honestly lost track of it, sorry @dangerousben

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.

4 participants