Switch branch Action #286
Switch branch Action #286
Conversation
|
Remove the backlog link from the description. Instead add some description of what the pull request solves.
|
| @@ -101,7 +102,8 @@ public class AbapGitView extends ViewPart implements IAbapGitRepositoriesView { | |||
| public static final String ID = "org.abapgit.adt.ui.views.AbapGitView"; //$NON-NLS-1$ | |||
|
|
|||
| protected TableViewer viewer; | |||
| protected Action actionRefresh, actionWizard, actionCopy, actionOpen, actionShowMyRepos, actionPullWizard, actionOpenRepository; | |||
| protected Action actionRefresh, actionWizard, actionCopy, actionOpen, actionShowMyRepos, actionPullWizard, actionOpenRepository, | |||
| actionSwitch; | |||
There was a problem hiding this comment.
Rename to actionSwitchBranch to avoid ambiguity with other possible actions like switching folder logic.
| public SwitchbranchAction(IViewPart view) { | ||
| super(Messages.AbapGitView_action_select_branch); | ||
| setToolTipText(Messages.AbapGitView_action_select_branch); | ||
| setImageDescriptor(AbstractUIPlugin.imageDescriptorFromPlugin(AbapGitUIPlugin.PLUGIN_ID, "icons/etool/compare_view.png")); //$NON-NLS-1$ |
There was a problem hiding this comment.
Need to check availability of any other icon.
| import org.eclipse.jface.dialogs.DialogPage; | ||
| import org.eclipse.jface.dialogs.TrayDialog; | ||
|
|
||
| public class AbapGitWizardPageBranchSelectionCredentials extends AbapGitWizardPageRepositoryAndCredentials { |
There was a problem hiding this comment.
It might be possible to remove this Class & reuse AbapGitWizardPageRepositoryAndCredentials.
This might require to modify the class a little.
To be specific, the following changes could be made to AbapGitWizardPageRepositoryAndCredentials :
- Add title as a parameter to the constructor.
- Set the title for the page in the constructor, based on this parameter
- Remove the existing logic for setting the title & pass the title to be set in the link & pull wizard and also for the new branch selection wizard.
Please try this out and list any issues with this approach.
| if (event.getCurrentPage() == AbapGitWizardBranchSelection.this.pageBranchAndPackage | ||
| && event.getTargetPage() == AbapGitWizardBranchSelection.this.pageCredentials) { | ||
| if (AbapGitWizardBranchSelection.this.pageBranchAndPackage.validateAll()) { | ||
| event.doit = false; | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Validation of content of current is not needed while going back in the wizard. This logic can be removed.
There was a problem hiding this comment.
Yeah true as we won't be navigating it back can remove this.
| public AbapGitWizardPageBranchSelection(IProject project, String destination, CloneData cloneData, Boolean pullAction) { | ||
| super(project, destination, cloneData, pullAction); | ||
| } |
There was a problem hiding this comment.
Also set a new title & description for the page. The parent page sets the title "Branch and Package Selection", but the branch selection wizard does not allow to select package.
Hence a new title & description should be set here.
Suggestion.
Title: Select Branch (Same as the credentials page)
Description: Select branch to switch from the dropdown
| @@ -0,0 +1,182 @@ | |||
| package org.abapgit.adt.ui.internal.repositories.wizards; | |||
There was a problem hiding this comment.
Move the classes to the package org.abapgit.adt.ui.internal.wizards.
Technically it's right to have repositories in the package name, but presently all wizards & related pages are only related to repositories view.
In future in case we have wizards also for staging view, in that case we can have this new package.
| public Object getProject() { | ||
| return this.project; | ||
| } | ||
|
|
||
| public Object getSelectedRepository() { | ||
| return this.selRepoData; | ||
| } |
There was a problem hiding this comment.
Are these methods needed and used anywhere? I don't see its usage.
| if (this.abapGitService == null) { | ||
| this.abapGitService = AbapGitUIServiceFactory.createAbapGitService(); | ||
| } |
There was a problem hiding this comment.
When the object is created, abapGitService will always be null right?
So, is this check required ?
There was a problem hiding this comment.
true that's not required, will remove it.
| } | ||
| getPackageAndRepoType(); | ||
|
|
||
| setWindowTitle(Messages.AbapGitView_action_select_branch); |
There was a problem hiding this comment.
The message variable can be renamed to AbapGitView_action_switch_branch.
| } catch (InvocationTargetException | InterruptedException e) { | ||
| // TODO Auto-generated catch block | ||
| e.printStackTrace(); | ||
| } | ||
| return true; |
There was a problem hiding this comment.
Show the error message on the page if possible.
|
@aarnapant-sap Please also test the solution in a ABAP Cloud system, involving transport requests. |
Switch Branch Functionality in AbapGit