Code review comment for lp:~javierder/bzr-eclipse/add-ignore

Revision history for this message
Guillermo Gonzalez (verterok) wrote :

>
> Hi, thanks for the review!
>

Hi, apologize the delay.

> About the "ResourceChangeCommand".
> I don't know if it applies, but resources are changed everytime any of those
> commands is executed.
sometimes what change is the the metadata associated with the branch, we could treat the ignored files as such, or as a resource modification, the difference is how the refresh is executed.

> If ignore is executed, .bzrignore is changed.
Actually .bzrignore might be changed, but ins't tracked so, isn't changing, let me digg a bit more about this. maybe we can improve this in a later branch.

> If addignored is executed, files are added, just like in AddCommand

Testing the ingored files dialog, I get NullPointer exceptions when unchecking one ignored file and clicking in ok.

java.lang.NullPointerException
at org.vcs.bazaar.eclipse.core.commands.AddIgnoredCommand.getFiles(AddIgnoredCommand.java:56)
at org.vcs.bazaar.eclipse.core.commands.AddIgnoredCommand.execute(AddIgnoredCommand.java:45)
at org.vcs.bazaar.eclipse.core.commands.MetaModifyCommand.run(MetaModifyCommand.java:29)
at org.vcs.bazaar.eclipse.ui.actions.IgnoredAction$1.execute(IgnoredAction.java:83)
at org.eclipse.ui.actions.WorkspaceModifyOperation$1.run(WorkspaceModifyOperation.java:104)
at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1800)
at org.eclipse.ui.actions.WorkspaceModifyOperation.run(WorkspaceModifyOperation.java:116)
at org.vcs.bazaar.eclipse.ui.actions.BzrAction$1.run(BzrAction.java:120)
at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)

looks like the problem is in IgnoredDialog.readUnchecked method.
fwiw, there is a list "uncheckedfiles" that's isn't used.
but the problem is that unchecked is populated with one null element.
possibly the error is in project.findMember((String)ignored[i].get(0)) as (String)ignored[i].get(0) is "<project>/<file>" and findMemeber looks for files/folders inside <project>

maybe using IBzrResource classes to show the files might simplify this, as all the code is already there. If you go with this approach, the Map<String, String> returned by the ignored command should be preprocessed before passing it to the dialog, in order to pass a Map<IBzrResource, String> to the dialog.

Regards.

« Back to merge proposal