Skip to content

Conversation

kasteph
Copy link
Contributor

@kasteph kasteph commented Feb 26, 2022

No description provided.

@kasteph kasteph force-pushed the kasteph/fix-task-gaps branch from b9c8016 to c267781 Compare March 1, 2022 19:14
@kasteph kasteph marked this pull request as ready for review March 1, 2022 19:16
@kasteph kasteph requested review from frrist, placer14 and hsanjuan March 1, 2022 19:18
@codecov-commenter
Copy link

codecov-commenter commented Mar 1, 2022

Codecov Report

Attention: Patch coverage is 56.25000% with 14 lines in your changes missing coverage. Please review.

Project coverage is 29.5%. Comparing base (2398245) to head (522f203).
Report is 261 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #884     +/-   ##
========================================
- Coverage    31.2%   29.5%   -1.7%     
========================================
  Files          39      39             
  Lines        3873    3726    -147     
========================================
- Hits         1209    1102    -107     
+ Misses       2518    2481     -37     
+ Partials      146     143      -3     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@placer14 placer14 left a comment

Choose a reason for hiding this comment

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

Apologies, but I'm not clear on what the motivation for this change is or what this intends to fix/improve. Is there an issue or could we include a description in the PR?

Most glaring here is the value of the dense rank isn't clear to me (yet). (Also, very sorry for the late review. 🙏)

@frrist frrist marked this pull request as draft March 8, 2022 18:36
@frrist
Copy link
Member

frrist commented Mar 8, 2022

This PR came about from a one-off conversation between @kasteph and myself. There isn't an issue tracking this, as it's mostly explorative work. The tentative next steps are refactoring the gap-find query into a single SQL function. @kasteph I've converted this to a draft to signal this, hope that's alright with you.

@kasteph
Copy link
Contributor Author

kasteph commented Mar 9, 2022

@frrist sounds good, thanks!

@frrist frrist linked an issue Mar 9, 2022 that may be closed by this pull request
@kasteph kasteph force-pushed the kasteph/fix-task-gaps branch from bbe1fb7 to a0ca674 Compare April 5, 2022 01:13
@frrist frrist force-pushed the kasteph/fix-task-gaps branch from 27be204 to 732d41d Compare April 5, 2022 22:09
Copy link
Member

@frrist frrist left a comment

Choose a reason for hiding this comment

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

Goood stuff! Excited to have this. Please address comments before merging.

@kasteph kasteph marked this pull request as ready for review April 10, 2022 01:51
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.

Implement Gap Find query as SQL function
4 participants