cosmicsilence / gradle-scalafix

Gradle plugin for Scalafix
BSD 3-Clause "New" or "Revised" License
27 stars 5 forks source link

SemanticDB version configurable via scalafix extension #42

Closed fsanjuan closed 3 years ago

fsanjuan commented 3 years ago

Fixes #40

fsanjuan commented 3 years ago

@marcelocenerine before merging this and releasing a new version I was just thinking about an improvement we could make in the scalafix extension. How about...

scalafix {
    semanticdb {
        autoConfigure = false
        version = '4.4.10'
    }
}

instead of

scalafix {
    autoConfigureSemanticdb = false
    semanticdbVersion = '4.4.10'
}
marcelocenerine commented 3 years ago

@marcelocenerine before merging this and releasing a new version I was just thinking about an improvement we could make in the scalafix extension. How about...

scalafix {
    semanticdb {
        autoConfigure = false
        version = '4.4.10'
    }
}

instead of

scalafix {
    autoConfigureSemanticdb = false
    semanticdbVersion = '4.4.10'
}

I think I like the idea. It would be a minor breaking change though. However, I'd expect that only a few (if any) users would be impacted as most rules require autoConfigureSemanticdb to be true (which is the default value and there is no reason to explicitly inform it). To mitigate that and any future breaking changes we may introduce, we could create a markdown file (migration guide?) with instructions on how upgrade and reference it in our README. What do you think?

fsanjuan commented 3 years ago

@marcelocenerine before merging this and releasing a new version I was just thinking about an improvement we could make in the scalafix extension. How about...

scalafix {
    semanticdb {
        autoConfigure = false
        version = '4.4.10'
    }
}

instead of

scalafix {
    autoConfigureSemanticdb = false
    semanticdbVersion = '4.4.10'
}

I think I like the idea. It would be a minor breaking change though. However, I'd expect that only a few (if any) users would be impacted as most rules require autoConfigureSemanticdb to be true (which is the default value and there is no reason to explicitly inform it). To mitigate that and any future breaking changes we may introduce, we could create a markdown file (migration guide?) with instructions on how upgrade and reference it in our README. What do you think?

Rather than a migration guide in a readme that I think many developers will not read, we can maintain both scalafix.semanticdb.autoconfigure and scalafix.autoconfigureSemanticdb during the next few versions. We can deprecate scalafix.autoconfigureSemanticdb and show a warning to the user indicating that extension field will not be available after version x.y and that they should use scalafix.semanticdb.autoconfigure instead.

That approach is similar to what the Gradle folks do when they deprecate features.

marcelocenerine commented 3 years ago

Sounds good 👍

fsanjuan commented 3 years ago

Sounds good 👍

I just pushed a 'preview'. I would expect it to fail in gradle 4.x

fsanjuan commented 3 years ago

LGTM. I made just some small tweaks to the README as autoConfigure was spelled as autoconfigure + a few other things. Please take a look and see if you agree.

Looks great! Thanks you very much! Merging now...