Re: [ice-build] [eclipse/ice] cacfa6: Turning off NeedsInfo test since t

["Billings, Jay Jay" <billingsjj@xxxxxxxx>]

So then you should delete it, not turn it off. If we aren't using it, removing it is better than disabling it because we don't need the dead code.

Jay

From my phone.

---

From: Alex McCaskey
Sent: Thursday, December 17, 2015 6:22 PM
To: Ice build notices ,Jay Billings
Subject: Re: [ice-build] [eclipse/ice] cacfa6: Turning off NeedsInfo test since that functionalit...

Yea I'm not really happy with just turning off the test either. But here's why I did it: 

The NeedsInfo flag was being used to tell the Client to display an extra info Form, so we could get the user name and password from the user for remote executions. Now that we use org.eclipse.remote services for our remote connections, those connections are stored in the Remote Development Preferences, as they should be. With the new RemoteAction class, all the interaction with those persisted connections is taken care of, so, for example, if RemoteExecutionAction needed an IRemoteConnection to open, it just calls getRemoteConnection(host) to get it. 

The issue is then, what if getRemoteConnection is called and there is not a connection stored for the given host name? Then you would need some way to prompt the user for username/password info so that you could create an IRemoteConnection, which would be a prime use case for the NeedsInfo flag. But there are service interfaces to acheive that from org.eclipse.remote, so I set up the RemoteAction to use those interfaces to display the Remote Connection Dialog and create the new connection, if it couldn't find one persisted. Now there should never not be a connection for subclasses of RemoteAction, so there isn't a need for NeedsInfo. That's what I meant in the commit message. I turned off the test mainly because I was lazy, it worked in Eclipse but not in the Maven build X-(

Of course, there are probably other use cases for NeedsInfo that I'm not thinking of, so yea I'm not happy with just turning off that test. But I do think that setting up RemoteAction this way is a good thing, in that it lets us leverage more Eclipse infrastructure and keeps us from reinventing the wheel. 

Thoughts?

Alex

On Thu, Dec 17, 2015 at 5:56 PM Jay Jay Billings <jayjaybillings@xxxxxxxxx> wrote:

Alex,

I think there's a problem with just turning off the test. We may need that capability in the future. What's the problem with the test?

Jay

On Dec 17, 2015 5:10 PM, "GitHub" <noreply@xxxxxxxxxx> wrote:

  Branch: refs/heads/mccaskey/joblauncherRefactor
  Home:   https://github.com/eclipse/ice
  Commit: cacfa6240cb6ffc8eb0bb067420eb1294bc02cc1
      https://github.com/eclipse/ice/commit/cacfa6240cb6ffc8eb0bb067420eb1294bc02cc1
  Author: amccaskey <mccaskeyaj@xxxxxxxx>
  Date:   2015-12-17 (Thu, 17 Dec 2015)

  Changed paths:
    M org.eclipse.ice.item.test/src/org/eclipse/ice/item/test/JobLauncherTester.java
    M org.eclipse.ice.item/META-INF/MANIFEST.MF

  Log Message:
  -----------
  Turning off NeedsInfo test since that functionality is not used anymore.

Signed-off-by: amccaskey <mccaskeyaj@xxxxxxxx>



_______________________________________________
ice-build mailing list
ice-build@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/ice-build

_______________________________________________
ice-build mailing list
ice-build@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/ice-build