Skip to content

[RSPEC-S1068] Remove unused private fields #2317

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

Closed
wants to merge 3 commits into from

Conversation

Pankraz76
Copy link
Contributor

@Pankraz76 Pankraz76 commented May 10, 2025

@@ -43,8 +43,6 @@ public final class CLIReportingUtils {
private static final long ONE_MINUTE = 60 * ONE_SECOND;

private static final long ONE_HOUR = 60 * ONE_MINUTE;

private static final long ONE_DAY = 24 * ONE_HOUR;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this might be API but seems unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Want to investigate, what its actually used for.

@Pankraz76 Pankraz76 force-pushed the RemoveUnusedPrivateFields branch from f1dbef5 to 1c1a395 Compare May 10, 2025 21:00

private static final long ONE_MINUTE = 60 * ONE_SECOND;

private static final long ONE_HOUR = 60 * ONE_MINUTE;
Copy link
Contributor Author

@Pankraz76 Pankraz76 May 10, 2025

Choose a reason for hiding this comment

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

@Pankraz76
Copy link
Contributor Author

Pankraz76 commented May 10, 2025

SUCCESS

image

@Pankraz76
Copy link
Contributor Author

kindly request some feedback.

@Pankraz76 Pankraz76 marked this pull request as ready for review May 12, 2025 07:58
@@ -28,10 +28,6 @@

class MavenITmng6401ProxyPortInterpolationTest extends AbstractMavenIntegrationTestCase {

private Proxy proxy;
Copy link
Contributor

Choose a reason for hiding this comment

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

These might be used in a weird way by reflection in the integration tests. I'm not sure in this case, but Maven does have a lot of private things that look unused but really are used like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes then its nice to have good TC to challenge or suppress.

@@ -47,24 +47,12 @@
*/
public class MyMojo extends AbstractMojo {

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the comment shows why and how this is used here. Please check this sort of thing before sending PRs from automated tools.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes im sorry.

order 100% test coverage or deletion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes its unused. Details does not help me. Im unaware.

From technical POV its seems obsolet.

If CI happy ship or test / use as its only test stage.

* Not used, just an offset to place reactorProjects in the middle.
* @parameter default-value="${project.build.directory}"
*/
private String outputDirectory2;
Copy link
Contributor Author

@Pankraz76 Pankraz76 May 12, 2025

Choose a reason for hiding this comment

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

seems c&p leftover.

desruisseaux and others added 2 commits May 12, 2025 15:26
The matcher returned by that method combines the effects of all includes and excludes.

When using the Maven syntax, escape the special characters [ ] { } \ before to delegate to the glob syntax.
Optimization: omit excludes that are unnecessary because they will never match a file accepted by includes.
This is especially useful when the default excludes are added, because there is a lot of them.

---------

Co-authored-by: VIP <[email protected]>
@Pankraz76
Copy link
Contributor Author

wip reopen

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.

4 participants