Skip to content

Conversation

@eth3lbert
Copy link
Contributor

Since we will respond with the default_version to the client, this PR always upsert the default_value for publish.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another approach would be to calculate from all versions. Since TopVersion already needs to calculate from all versions, we could consider pre-sorting the versions and providing methodologies for TopVersion and default_versions to determine values from the sorted versions.

@codecov
Copy link

codecov bot commented Oct 22, 2024

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.88%. Comparing base (ba72798) to head (a5d24b3).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/controllers/krate/publish.rs 96.42% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9722      +/-   ##
==========================================
- Coverage   88.88%   88.88%   -0.01%     
==========================================
  Files         289      289              
  Lines       29552    29568      +16     
==========================================
+ Hits        26268    26282      +14     
- Misses       3284     3286       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bors
Copy link
Contributor

bors commented Oct 22, 2024

☔ The latest upstream changes (presumably #9723) made this pull request unmergeable. Please resolve the merge conflicts.

@eth3lbert eth3lbert force-pushed the default-version-for-publish branch from 338ff27 to 095cba4 Compare October 22, 2024 11:24
@eth3lbert eth3lbert marked this pull request as ready for review October 22, 2024 11:26
@eth3lbert eth3lbert requested a review from Turbo87 October 22, 2024 11:28
@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-backend ⚙️ labels Oct 22, 2024
@eth3lbert eth3lbert force-pushed the default-version-for-publish branch from 095cba4 to 3d5d899 Compare October 22, 2024 11:54
@eth3lbert eth3lbert force-pushed the default-version-for-publish branch from 3d5d899 to 9e6a5d1 Compare October 22, 2024 12:02
The value is determined by comparing the existing `default_value` to the
published version. This could potentially result in upserting an
outdated version. A subsequent background job is necessary to ensure
correctness and eventual consistency.
@eth3lbert eth3lbert force-pushed the default-version-for-publish branch from 9e6a5d1 to a5d24b3 Compare October 22, 2024 12:29
Copy link
Member

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

:)

@Turbo87 Turbo87 enabled auto-merge October 22, 2024 12:39
@Turbo87 Turbo87 merged commit 3e9d6e3 into rust-lang:main Oct 22, 2024
8 checks passed
@eth3lbert eth3lbert deleted the default-version-for-publish branch October 22, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants