-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Rework PlainJava project type and Deprecate SettableValueProvider #9634
Conversation
# Conflicts: # ide/che-core-ide-app/src/main/java/org/eclipse/che/ide/context/AppContextImpl.java
…roject' volume defined
…re no 'project' volume defined" This reverts commit 127a79e.
…roject' volume defined
…ode, remove highlighter from the source folder field
ci-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request has a lot of commented code lines, do we need all of them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you get rid of all the commented code, please?
@@ -145,7 +143,7 @@ private void setAttribute(String attrId, List<String> value) { | |||
} | |||
|
|||
private boolean isCoordinatesCompleted() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method looks redundant since there's just one place where isCoordinatesCompleted()
is called.
delegate.onBrowseSourceButtonClicked(); | ||
} | ||
}); | ||
browseSourceBtn.setVisible(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
browseSourceBtn
is always hidden so I guess it's not needed anymore. Should it be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree but lets do it later )
ci-test build report: |
comments deleted |
…lipse-che#9634) * Adding pipeline * del jenkinsfile * Make exec agent not to use setsid for other than Linux envs * revert pom.xml * goformat * get projects folder from workkspace configuration * Merge remote-tracking branch 'upstream/master' * clean code * clean code * Fix calculation Projects Root so it wont cause NPE if there are no 'project' volume defined * Revert "Fix calculation Projects Root so it wont cause NPE if there are no 'project' volume defined" This reverts commit 127a79e. * Fix calculation Projects Root so it wont cause NPE if there are no 'project' volume defined * replace provided attribute values with stored (src, out) in PlainJavaProjectType * fix PlainJavaProjectType for getting rid and deprecate using SettableValueProvider * remove comented code * hide brouse source folder button, set source folder field as disabled * hide the browse source button, set source folder field to read only mode, remove highlighter from the source folder field * adapt test for current changes on UI, set save button to enable state * apply formatting * fix order of steps in the test
What does this PR do?
Make PlainJavaExtension's ValueProvider read-only (if was the last PT using settable one) and deprecate SettableValueProvider.
What issues does this PR fix or reference?
To simplify project type framework getting rid of not useful SettableValueProvider
Release Notes
N/A
Docs PR
N/A