vscode-java-dependency icon indicating copy to clipboard operation
vscode-java-dependency copied to clipboard

ux - display maven and gradle dependencies with pattern 'groupId:artifactId:version '

Open mamilic opened this issue 1 year ago • 5 comments

This MR referes to the issue, https://github.com/microsoft/vscode-java-dependency/issues/857

mamilic avatar Oct 16 '24 09:10 mamilic

Hi @jdneo , this is my view of the implementation for https://github.com/microsoft/vscode-java-dependency/issues/857, though I am not fully satisfied with changes, as I had to introduce getLabel() method to ExplorerNode. It seems that this only works for Maven, as when gradle project is loaded, the jdtls is not recognizing dependencies as Gradle, instead it is. :/ image

Could you please take a look and let me know what I might have done better? Also, is there a requirement to test his functionality?

mamilic avatar Oct 16 '24 09:10 mamilic

@microsoft-github-policy-service agree

mamilic avatar Oct 18 '24 11:10 mamilic

Overall looks good to me. Are you willing to add a test case in the maven-suite/projectView.test.ts? I can also handle that you have no time to do that.

jdneo avatar Oct 29 '24 02:10 jdneo

Overall looks good to me. Are you willing to add a test case in the maven-suite/projectView.test.ts? I can also handle that you have no time to do that.

I'm definitely willing to add a test case. It would be a great learning opportunity for me, though it might take some time. That said, it depends on how quickly we’re looking to merge this. Let me know what works best!

mamilic avatar Oct 29 '24 05:10 mamilic

I'm not hurry, just take your time 😉

jdneo avatar Oct 29 '24 07:10 jdneo

@jdneo, 2 tests are failing, though I am not sure if that is intended, as the latest build from main branch reflect the same result, https://github.com/microsoft/vscode-java-dependency/actions/runs/10659297304/job/29541618151

mamilic avatar Oct 30 '24 12:10 mamilic

It's unlikely the failures are caused by this PR. Please give me some time to do some investigation (I was occupied by some other stuff recently).

If I cannot get some finding I'll merge this PR first.

jdneo avatar Oct 30 '24 12:10 jdneo

I tested on my mac but I cannot reproduce it.

jdneo avatar Oct 31 '24 04:10 jdneo

@mamilic Thanks for your contribution!

I appended a commit to pass the test. I guess it's related with some timing issue. Since gradle project needs to download gradle distribution during import in CI jobs. It will take longer time.

jdneo avatar Oct 31 '24 07:10 jdneo

@jdneo, thank you for your patience and guidence on this issue, and do apologies for taking so long. Next step is to enable this for gradle.

mamilic avatar Oct 31 '24 07:10 mamilic