search

scoverage / gradle-scoverage

A plugin to enable the use of Scoverage in a gradle Scala project
Apache License 2.0
53 stars 38 forks source link

Fix deprecation warnings about undeclared dependency (round 2) #167

Closed eyalroth closed 3 years ago

eyalroth commented 3 years ago

Fixes #163 (again).

This PR achieves the following:

  1. Upgrade the gradle wrapper to 7.1.1 (to get the deprecation warnings).
  2. Add a CI job that fails on any warning, but doesn't fail the overall build.
  3. Fix the deprecation warning by making the report and aggregation tasks declare their "sources" only on the actual source files and not the entire project directory. This changes the path of the report files (removes the src/main/scala prefix).

The changes in ScoverageWriter.java make use of the Seq constructors of the various writers imported from the scalac plugin, while making it possible to compile this plugin once and be able to work with either scala (and scalac plugin) 2.12 or 2.13.

eyalroth commented 3 years ago

@maiflai Let me know if this is possible to merge this PR even though one of the check fails (more deprecation warnings).

If not, then I'll change the CI configuration to not fail the "fail on warning" job but only fail the gradle step.

maiflai commented 3 years ago

interesting - not sure what the impact on the build status will be if we move to having a matrix build.

perhaps we should have a separate action if we don't plan to fix this immediately?

also, could we update to 7.2 which I think is now available?

eyalroth commented 3 years ago

interesting - not sure what the impact on the build status will be if we move to having a matrix build.

perhaps we should have a separate action if we don't plan to fix this immediately?

The overall build action is successful, as can be seen here, but I don't know if it blocks a PR from merging or not. This is most likely configurable in the repository settings, under the protection rule for master.

I think it would be better if the PR would show that a check has failed even though it doesn't block it, as long as we fix this check now (I'm planning a separate PR) and try and keep it in the green in the future.

maiflai commented 3 years ago

I think that it can be merged without a successful action. I think that it would need someone with greater powers than me to change this, I don't have access to repository settings.

Can the other PR be merged now?

eyalroth commented 3 years ago

@maiflai Then I think that's the ideal situation; i.e, when the build with warnings fails it shows up in the PR as a failed check but doesn't block it from being merged.

And on that note, yes this PR can be merged and then #169 as well.