Commit for issue #10640

Coordinator
Sep 18, 2009 at 2:43 AM
I was reviewing this and had some questions on the commit:

1) I'm not sure what the change in the import form does (the scm bit)?

2) I don't see anything removing the warning dialog that can occur
when you got a non-portable reference before?

3) It ended up using the default group ID and artifact ID to determine
if a project is in the workspace, which is contrary to the IRC
discussion we had yesterday. This is likely to be incorrect if they
don't use the default when importing. The loop through the projects
should be obtaining the values from the actual POM and checking
against those. Are they stored in memory anywhere?

Thanks,
Brett
Coordinator
Sep 18, 2009 at 3:07 AM

Hi Brett,

1) I'm not sure what the change in the import form does (the scm bit)?

- It was unintentional that the scm bug fix on a different issue was included on this commit.

2) I don't see anything removing the warning dialog that can occur
when you got a non-portable reference before?

There was an added condition statement that would skip the warning message on intra project dependencies.


3) It ended up using the default group ID and artifact ID to determine
if a project is in the workspace, which is contrary to the IRC
discussion we had yesterday. This is likely to be incorrect if they
don't use the default when importing. The loop through the projects
should be obtaining the values from the actual POM and checking
against those. Are they stored in memory anywhere?

Yes this was a misunderstanding on the implementation. This will be revised, I have already discussed with Jahn regarding the implementation and it will be basing on the actual pom file. Also there is a missing use case that we are discussing, in the event that there is no pom(the project is not yet imported to npanday) When the project is actually imported it should be treated with the same effect as add maven artifact.

 

Thanks,

Joe


Editor
Sep 18, 2009 at 3:31 AM

also for #3, looping through the projects in VS was intentional since we need a real-time list of projects. getting it from modules in the parent POM might result to outdated list of projects

 

Jahn

Coordinator
Sep 18, 2009 at 3:56 AM

On 18/09/2009, at 1:07 PM, jocaba wrote:

From: jocaba

Hi Brett,

1) I'm not sure what the change in the import form does (the scm bit)?

- It was unintentional that the scm bug fix on a different issue was included on this commit.


which issue is that related to, for further reference?

2) I don't see anything removing the warning dialog that can occur
when you got a non-portable reference before?

- There was an added condition statement that would skip the warning message on intra project dependencies.


under what circumstances does the error occur then? Is it if a reference is added on a DLL in the filesystem but not in the solution, the GAC, and not able to be resynced?

I can't see how adding a system dependency ever makes sense, since it's always possible to be in a different place on another machine.


3) It ended up using the default group ID and artifact ID to determine
if a project is in the workspace, which is contrary to the IRC
discussion we had yesterday. This is likely to be incorrect if they
don't use the default when importing. The loop through the projects
should be obtaining the values from the actual POM and checking
against those. Are they stored in memory anywhere?

- Yes this was a misunderstanding on the implementation. This will be revised, I have already discussed with Jahn regarding the implementation and it will be basing on the actual pom file.

ok, So Jahn said:

On 18/09/2009, at 1:31 PM, jnpaclibar wrote:

From: jnpaclibar

also for #3, looping through the projects in VS was intentional since we need a real-time list of projects. getting it from modules in the parent POM might result to outdated list of projects

I agree with the looping through the projects - I was suggesting that the basis for comparison within there should be the POM-specified coordinate instead of the company name and project name which are only defaults.

Also there is a missing use case that we are discussing, in the event that there is no pom(the project is not yet imported to npanday) When the project is actually imported it should be treated with the same effect as add maven artifact.


Agreed - in that case it would then use the defaults and the later import would need to check the reference is correct?

- Brett

Coordinator
Sep 18, 2009 at 4:07 AM

2) I don't see anything removing the warning dialog that can occur
when you got a non-portable reference before?

- There was an added condition statement that would skip the warning message on intra project dependencies.

under what circumstances does the error occur then? Is it if a reference is added on a DLL in the filesystem but not in the solution, the GAC, and not able to be resynced?

I can't see how adding a system dependency ever makes sense, since it's always possible to be in a different place on another machine.

I agree that adding a system dependency would not be logical because it will not be the same on other dev machines, However there is a behavior in Visual Studio that would allow for the user to browse the project and then use the .dll of this project. We are just anticipating that behavior and in turn will be prompting the user that all though it can build locally on his machine it will not be a portable project for other devs.

Editor
Sep 18, 2009 at 4:21 AM
brettporter wrote:

On 18/09/2009, at 1:07 PM, jocaba wrote:

From: jocaba

Hi Brett,

1) I'm not sure what the change in the import form does (the scm bit)?

- It was unintentional that the scm bug fix on a different issue was included on this commit.


which issue is that related to, for further reference?

i just created an issue for this here. this was discovered on our 1.0.2 release but was regarded as minor

Coordinator
Sep 18, 2009 at 4:28 AM

On 18/09/2009, at 2:07 PM, jocaba wrote:

under what circumstances does the error occur then? Is it if a reference is added on a DLL in the filesystem but not in the solution, the GAC, and not able to be resynced?

I can't see how adding a system dependency ever makes sense, since it's always possible to be in a different place on another machine.

I agree that adding a system dependency would not be logical because it will not be the same on other dev machines, However there is a behavior in Visual Studio that would allow for the user to browse the project and then use the .dll of this project. We are just anticipating that behavior and in turn will be prompting the user that all though it can build locally on his machine it will not be a portable project for other devs.

This is probably a separate issue now - but perhaps we can investigate that down the track. I'd ask - do the projects they browse to also have a potential Maven identifier that we can rely on instead of using the system scope? And if it is not found in the repositories, then present an error instead of adding the system scope? (or maybe offer a choice).

- Brett
Coordinator
Sep 18, 2009 at 4:40 AM

This is probably a separate issue now - but perhaps we can investigate that down the track. I'd ask - do the projects they browse to also have a potential Maven identifier that we can rely on instead of using the system scope? And if it is not found in the repositories, then present an error instead of adding the system scope? (or maybe offer a choice).

This browse option is not necessarily in a project format, it could simply be just a .dll by itself placed in the desktop so we would be having a hard time to figure out the basis for a maven identifier. This feature has been supported from npanday 1.0 since there are some users that pointed out that they want to have it like that for local development purposes.

-Joe

Coordinator
Sep 18, 2009 at 5:04 AM

On 18/09/2009, at 2:40 PM, jocaba wrote:

From: jocaba

This is probably a separate issue now - but perhaps we can investigate that down the track. I'd ask - do the projects they browse to also have a potential Maven identifier that we can rely on instead of using the system scope? And if it is not found in the repositories, then present an error instead of adding the system scope? (or maybe offer a choice).

This browse option is not necessarily in a project format, it could simply be just a .dll by itself placed in the desktop so we would be having a hard time to figure out the basis for a maven identifier. This feature has been supported from npanday 1.0 since there are some users that pointed out that they want to have it like that for local development purposes.


Right... sorry, that's what I was getting at previously. We can probably keep that then :)

- Brett