-
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
CHE-2456: Svn switch action #2758
Conversation
Build # 686 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/686/ to view the results. |
@vinokurig |
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.
You should rework this to support remote operations with authorization dialog. See this: https://github.com/eclipse/che/blob/11036470a03e3af875cc977f424cb0019ec76db5/plugins/plugin-svn/che-plugin-svn-ext-ide/src/main/java/org/eclipse/che/plugin/svn/ide/common/SubversionActionPresenter.java#L149-L174
Build # 687 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/687/ to view the results. |
protected void onClose() { } | ||
|
||
@UiHandler("switchToBranch") | ||
public void onSwitchToBranchClicked(final ClickEvent event) { delegate.onSwitchToBranchChanged(); } |
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.
Please format this method body in the same way to other
public void onWorkingCopyDepthChanged(final ChangeEvent event) { | ||
delegate.onWorkingCopyDepthChanged(); | ||
} | ||
|
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.
remove extra lines
* @param request | ||
* the switch request | ||
* @return the response | ||
* @throws SubversionException |
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.
Looks like it throws ApiException
* | ||
* @return the response containing target children | ||
* * @param request |
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.
remove extra *
request.getUsername(), | ||
request.getPassword()); | ||
|
||
return DtoFactory.getInstance().createDto(ListResponse.class) |
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.
DtoFactory.newDto(ListResponse)...
?
Build # 690 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/690/ to view the results. |
/** Show the view. */ | ||
void showWindow(); | ||
|
||
void setRootNode(SvnNode rootNode); |
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.
javadoc?
}).catchError(new Operation<PromiseError>() { | ||
@Override | ||
public void apply(PromiseError error) throws OperationException { | ||
notificationManager.notify("Error retrieving svn info", error.getMessage(), FAIL, EMERGE_MODE); |
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.
Please move the messages to locale constants
import org.eclipse.che.dto.shared.DTO; | ||
|
||
import javax.validation.constraints.NotNull; | ||
|
||
/** |
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.
Please add some info to this javadoc
Build # 692 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/692/ to view the results. |
@@ -100,7 +100,7 @@ | |||
SVGResource status(); | |||
|
|||
@Source("actions/switch.svg") | |||
SVGResource switchLocation(); | |||
SVGResource sw(); |
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.
sw ????
return projectUri + "/trunk"; | ||
|
||
} else if (switchView.isSwitchToBranch()) { | ||
return projectUri + "/branches/" + (isNullOrEmpty(switchView.getSwitchToLocation()) ? "" : switchView.getSwitchToLocation()); |
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.
move switchView.getSwitchToLocation() to var in case it used 4 times here
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.
possibly if switchView.getSwitchToLocation() == null we don't need ending "/"
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.
Actually it works in both cases.
this.resources = resources; | ||
this.constants = constants; | ||
|
||
this.ensureDebugId("svn-switch-window"); |
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.
a lot of "this" i think :)
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.
But it looks nicer :)
Build # 694 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/694/ to view the results. |
Build # 701 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/701/ to view the results. |
What does this PR do?
Add svn switch action.
What issues does this PR fix or reference?
#2456
New behavior
Ability to switch to trunk/branch/tag from svn menu