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

check if the version of the migrated artifact exists #2455

Merged
merged 4 commits into from
Jan 22, 2022

Conversation

mzuehlke
Copy link
Member

@mzuehlke mzuehlke commented Jan 8, 2022

Check if the version of the migrated artifact exists using the versionCache. As now the version of the migrated artifact is detected ArtifactChange#initialVersion is no longer needed.

fixes #2450

This should prevent problems like in #2447

@codecov
Copy link

codecov bot commented Jan 8, 2022

Codecov Report

Merging #2455 (6b1c066) into master (dbf751d) will increase coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2455      +/-   ##
==========================================
+ Coverage   81.05%   81.17%   +0.11%     
==========================================
  Files         142      142              
  Lines        2460     2475      +15     
  Branches       39       44       +5     
==========================================
+ Hits         1994     2009      +15     
  Misses        466      466              
Impacted Files Coverage Δ
...asteward/core/update/artifact/ArtifactChange.scala 100.00% <ø> (ø)
...ore/update/artifact/ArtifactMigrationsFinder.scala 87.50% <ø> (-4.81%) ⬇️
...scala/org/scalasteward/core/update/UpdateAlg.scala 100.00% <100.00%> (ø)

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 dbf751d...6b1c066. Read the comment docs.

@fthomas
Copy link
Member

fthomas commented Jan 11, 2022

👍

I haven't had time to look into this PR in detail yet but was wondering if we could get rid of ArtifactChange#initialVersion with this change. If Scala Steward queries all versions of the migrated artifact, why not let itself decide what the next suitable version is?

@mzuehlke
Copy link
Member Author

I like the the idea of removing ArtifactChange#initialVersion. I adapted the PR and it makes it simpler. Take a look whenever it fits.

Copy link
Member

@fthomas fthomas left a comment

Choose a reason for hiding this comment

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

This LGTM. Thank you @mzuehlke!

@fthomas fthomas merged commit 17cf219 into master Jan 22, 2022
@fthomas fthomas added the enhancement New feature or request label Jan 22, 2022
@fthomas fthomas added this to the 0.14.0 milestone Jan 22, 2022
@mzuehlke mzuehlke deleted the check_migrated_artifact branch January 22, 2022 16:19
fthomas added a commit that referenced this pull request Jan 22, 2022
This adds a new default file for artifacts migrations that does not
contain the `initialVersion` fields. Since older Scala Steward instances
load the list of artifact migrations from this repository, removing
fields is a backward-incompatible change so that we cannot remove them
in the old default file.

See #2455 and #2472 for context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check if migrated artifacts are released before creating PRs
2 participants