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

chore: minor tweaks as modev takes over streaming updates #8909

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

rb-determined-ai
Copy link
Contributor

  • add DESIGN doc back into code
  • fix some typos in comments
  • rely on defer in the context cancelation function
  • eliminate unnecessary/unhelpful wrapper function
  • use time.Ticker instead of time.After

@rb-determined-ai rb-determined-ai requested a review from a team as a code owner February 27, 2024 20:32
@cla-bot cla-bot bot added the cla-signed label Feb 27, 2024
@rb-determined-ai rb-determined-ai requested review from jgongd and removed request for salonig23 February 27, 2024 20:32
Copy link

netlify bot commented Feb 27, 2024

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 0f7be7a
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/65de4cc37e13c5000838eb80

Copy link

codecov bot commented Feb 27, 2024

Codecov Report

Attention: Patch coverage is 58.33333% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 47.53%. Comparing base (2689b0b) to head (0f7be7a).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8909      +/-   ##
==========================================
+ Coverage   44.44%   47.53%   +3.09%     
==========================================
  Files        1038     1066      +28     
  Lines      168477   170228    +1751     
  Branches     2235     2236       +1     
==========================================
+ Hits        74873    80915    +6042     
+ Misses      93446    89155    -4291     
  Partials      158      158              
Flag Coverage Δ
backend 43.36% <58.33%> (+<0.01%) ⬆️
harness 63.77% <ø> (+13.30%) ⬆️
web 42.50% <ø> (ø)

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

Files Coverage Δ
master/internal/stream/projects.go 75.45% <ø> (ø)
master/internal/stream/supervisor.go 0.00% <ø> (ø)
master/internal/stream/publisher.go 63.47% <58.33%> (+0.57%) ⬆️

... and 154 files with indirect coverage changes

- add DESIGN doc back into code
- fix some typos in comments
- rely on defer in the context cancelation function
- eliminate unnecessary/unhelpful wrapper function
- use time.Ticker instead of time.After
@rb-determined-ai rb-determined-ai merged commit 94c7bfe into main Feb 28, 2024
71 of 86 checks passed
@rb-determined-ai rb-determined-ai deleted the rb/streaming-updates-tweaks branch February 28, 2024 20:15
maxrussell pushed a commit that referenced this pull request Mar 21, 2024
- add DESIGN doc back into code
- fix some typos in comments
- rely on defer in the context cancelation function
- eliminate unnecessary/unhelpful wrapper function
- use time.Ticker instead of time.After
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants