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

Don't use versions that only consist of a hash. #2498

Merged
merged 3 commits into from
Feb 2, 2022
Merged

Conversation

mzuehlke
Copy link
Member

@mzuehlke mzuehlke commented Feb 1, 2022

@fthomas do you think this is too drastic

Fixes #2483

@codecov
Copy link

codecov bot commented Feb 1, 2022

Codecov Report

Merging #2498 (ce59944) into main (c6c77af) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2498      +/-   ##
==========================================
+ Coverage   81.19%   81.22%   +0.02%     
==========================================
  Files         142      142              
  Lines        2489     2514      +25     
  Branches       55       48       -7     
==========================================
+ Hits         2021     2042      +21     
- Misses        468      472       +4     
Impacted Files Coverage Δ
...ain/scala/org/scalasteward/core/data/Version.scala 100.00% <100.00%> (ø)
...rg/scalasteward/core/edit/hooks/HookExecutor.scala 90.27% <0.00%> (-3.60%) ⬇️

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 c6c77af...ce59944. Read the comment docs.

@fthomas
Copy link
Member

fthomas commented Feb 1, 2022

do you think this is too drastic

I'm not sure. It would prevent updates with very long versions that only consists of digits and are not dates. For example, a bump like 10000000 -> 20000000 would be prevented. If we change it slightly from v.isOnlyHash to isOnlyHash =!= v.isOnlyHash (or even !isOnlyHash && v.isOnlyHash), we would allow this bump but still prevent things like 1.4.12 -> 1032048a.

Use hash versions, if current version is only hash, too
@mzuehlke mzuehlke merged commit 0620824 into main Feb 2, 2022
@mzuehlke mzuehlke deleted the prevent-hash-verions branch February 2, 2022 07:52
@fthomas fthomas added the enhancement New feature or request label Feb 2, 2022
@fthomas fthomas added this to the 0.15.0 milestone Feb 2, 2022
@fthomas
Copy link
Member

fthomas commented Feb 2, 2022

Here is the next challenge: spotify/scio#4235 - a bump from 1.4.12 to 7d2bf0af+20171218-1522 ;-)

Maybe we should redefine containsHash such that it does not require a leading separator. 🤔

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.

Prevent updates from "normal" versions to hashes
2 participants