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

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

Merged

Conversation

eyalroth
Copy link
Contributor

@eyalroth eyalroth commented Aug 28, 2021

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 eyalroth force-pushed the fix-deprecation-undeclared-dependency branch from 6d99342 to 2761bb2 Compare August 28, 2021 14:13
@eyalroth eyalroth force-pushed the fix-deprecation-undeclared-dependency branch from 2761bb2 to d7da6b1 Compare August 28, 2021 14:19
…red dependency between report task and test tasks outputs
@eyalroth eyalroth marked this pull request as ready for review August 28, 2021 18:08
@eyalroth
Copy link
Contributor Author

@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.

@eyalroth eyalroth changed the title (CI test) Add CI build step that fails on warnings (but doesn't fail build) Fix deprecation warnings about undeclared dependency (round 2) Aug 28, 2021
@maiflai
Copy link
Contributor

maiflai commented Aug 30, 2021

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
Copy link
Contributor Author

eyalroth commented Sep 4, 2021

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
Copy link
Contributor

maiflai commented Sep 5, 2021

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
Copy link
Contributor Author

eyalroth commented Sep 7, 2021

@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.

@maiflai maiflai merged commit 73bd170 into scoverage:master Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gradle 7 incompatibility due to implicit dependency
2 participants