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

Implementation of #1251 create multiple types at once #1652

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nettozahler
Copy link
Contributor

What it does

Implementation of #1251 (create multiple types at once)

How to test

  • Run tests: NewTypeWizardTest, NewTypeWizardTest17
  • Manually: Open the "New Java Class" (or interface, record, annotation, enum) dialog. Insert multiple type names separated by semicolon e. g. A;B;C and press "Finish". Review the newly created types.

TODO / open questions

  • Since I have changed some existing method signatures I got problem markers regarding "the method xyz has been removed". Does this mean I need a version bump? @jjohnstn please advise here.
  • Should we be defensive and limit the maximum amount of created elements to... maybe 20?
  • @jukzi If Consider to add support to create multiple types at once through the Dialog Window #1251 gets resolved I think there are no more issues labeled "good first issue" so we need some more :-)
  • Could this feature be relevant for "New and Noteworthy"?

Author checklist

@jukzi
Copy link
Contributor

jukzi commented Sep 17, 2024

@nettozahler please fix the build errors (you must not change existing API! You can add new API though), sign ECA and quash your commits to a single commit

@nettozahler nettozahler force-pushed the 1251-create-multiple-types-at-once branch from 1c59008 to 7b12f4c Compare September 17, 2024 09:16
@nettozahler
Copy link
Contributor Author

nettozahler commented Sep 17, 2024

Fixed:

  • ECA issue (was caused by missing e-mail in my GIT configuration)
  • squashed the commits to a single one

@@ -758,14 +758,16 @@ private void internalSetJUnit4(boolean isJUnit4) {
}

@Override
protected String constructCUContent(ICompilationUnit cu, String typeContent, String lineDelimiter) throws CoreException {
Copy link
Contributor

Choose a reason for hiding this comment

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

you must not change the Signature of methods that are API. or you get
The method org.eclipse.jdt.junit.wizards.NewTestSuiteWizardPage.constructCUContent(ICompilationUnit, String, String) has been removed
You can however add and additional method and make sure that the old API still works.

@@ -3529,6 +3789,7 @@ public IStatus getSuperInterfaceStatus() {
}

/**
* Getter.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please avoid such Boiler plate comments and refactorings. they make this commit unnecessary complicated to review.

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.

2 participants