Skip to content

Conversation

@darylrobbins
Copy link
Contributor

@darylrobbins darylrobbins commented Apr 19, 2024

What's changed?

  • AddMissingMethodImplementation will no longer add methods to interfaces
  • Added tests for AddMissingMethodImplementation

What's your motivation?

Fixes #459

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

- Add tests for AddMissingMethodImplementation

Fixes openrewrite#459
- Add tests for AddMissingMethodImplementation

Fixes openrewrite#459
@timtebeek timtebeek self-requested a review April 20, 2024 12:27
@timtebeek timtebeek added the bug Something isn't working label Apr 20, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • src/main/java/org/openrewrite/java/migrate/AddMissingMethodImplementation.java
    • lines 18-19
    • lines 31-30

darylrobbins and others added 2 commits April 20, 2024 08:30
…lementationTest.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…lementationTest.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • src/main/java/org/openrewrite/java/migrate/AddMissingMethodImplementation.java
    • lines 18-19
    • lines 31-30

@darylrobbins
Copy link
Contributor Author

darylrobbins commented Apr 20, 2024

@timtebeek UpdateBeanManagerMethodsTest > fireEvent() is failing for both of the PR's so this doesn't look like anything to do with my changes. FWIW, the tests all pass for me locally.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • src/main/java/org/openrewrite/java/migrate/AddMissingMethodImplementation.java
    • lines 18-19
    • lines 31-30

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • src/main/java/org/openrewrite/java/migrate/AddMissingMethodImplementation.java
    • lines 18-19
    • lines 31-30

Copy link
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Thanks a lot for adding the tests here in addition to fixing the underlying issue. It seems our automated PR review was thrown off a bit when correcting the imports; we expect to see the static imports added after any regular imports, whereas your IDE settings seem to put them first. No problem at all; we'll fix that on PRs as they come in. Glad to have you on board here!

@timtebeek timtebeek merged commit c9bd305 into openrewrite:main Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

AddMissingMethodImplementation adds method implementations to Interfaces

2 participants