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

Introduce error-prone-experimental module #1003

Merged
merged 2 commits into from
Feb 10, 2024

Conversation

rickie
Copy link
Member

@rickie rickie commented Jan 28, 2024

❗ This PR is on top of #1000. ❗

@rickie rickie added this to the 0.15.0 milestone Jan 28, 2024
@rickie rickie requested a review from Stephan202 January 28, 2024 13:40
<artifactId>error-prone-experimental</artifactId>

<name>Picnic :: Error Prone Support :: Experimental</name>
<description>Experimental Error Prone checks.</description>
Copy link
Member Author

Choose a reason for hiding this comment

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

Had no better idea as my alternatives all got a bit too long.

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 force-pushed the rossendrijver/ep-utilities-module branch from 270c65c to 947914a Compare February 4, 2024 17:17
@Stephan202 Stephan202 force-pushed the rossendrijver/ep_experimental_module branch from f3360c6 to ba721eb Compare February 4, 2024 17:25
Copy link

github-actions bot commented Feb 4, 2024

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Rebased and added a commit. Suggested commit message:

Introduce `error-prone-experimental` module (#1003)

And move the known-incomplete `MethodReferenceUsage` check to it.

@@ -24,7 +24,7 @@
* A {@link BugChecker} that flags lambda expressions that can be replaced with a method reference
* of the form {@code T.class::isInstance}.
*
* @see MethodReferenceUsage
* <p>See the `MethodReferenceUsage` check in the `error-prone-experimental` module.
Copy link
Member

Choose a reason for hiding this comment

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

Here and below: instead of backticks we should use {@code ..}. But since there's a corresponding XXX comment: let's drop.

Comment on lines 3 to 6
This module contains Error Prone checks that are currently under development or
evaluation. These checks may be works-in-progress or have uncertain impacts on
code. Having this module allows for controlled experimentation and refinement
before integration into production environments.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This module contains Error Prone checks that are currently under development or
evaluation. These checks may be works-in-progress or have uncertain impacts on
code. Having this module allows for controlled experimentation and refinement
before integration into production environments.
This module contains Error Prone checks that are currently under development or
evaluation. These checks may be works-in-progress or have uncertain impact on
code. Having this module allows for controlled experimentation and refinement
before integration into production environments.

Comment on lines 63 to 72
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jspecify</groupId>
<artifactId>jspecify</artifactId>
<scope>provided</scope>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

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

There are a few more unused dependencies here; will drop.

Copy link
Member

Choose a reason for hiding this comment

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

We should change the package name, in preparation for one day adopting the Java module system (which doesn't allow packages to be split between modules).

I'm proposing tech.picnic.errorprone.experimental.bugpatterns.

Copy link

github-actions bot commented Feb 4, 2024

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link
Contributor

@mohamedsamehsalah mohamedsamehsalah left a comment

Choose a reason for hiding this comment

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

🧑‍🔬 🧑‍🔬 🧑‍🔬

@rickie rickie force-pushed the rossendrijver/ep-utilities-module branch from 947914a to 730c089 Compare February 9, 2024 13:40
Base automatically changed from rossendrijver/ep-utilities-module to master February 9, 2024 15:09
@rickie rickie force-pushed the rossendrijver/ep_experimental_module branch from eb21d65 to f1be860 Compare February 9, 2024 17:22
@rickie
Copy link
Member Author

rickie commented Feb 9, 2024

Rebased and resolved conflict. Will merge once 🍏.

Copy link

github-actions bot commented Feb 9, 2024

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202 Stephan202 force-pushed the rossendrijver/ep_experimental_module branch from f1be860 to c55ccfb Compare February 10, 2024 09:42
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link

@Stephan202 Stephan202 merged commit 6195ede into master Feb 10, 2024
16 checks passed
@Stephan202 Stephan202 deleted the rossendrijver/ep_experimental_module branch February 10, 2024 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants