Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: get correct project name #95

Merged
merged 1 commit into from
Sep 17, 2019

Conversation

sfat
Copy link
Contributor

@sfat sfat commented Sep 1, 2019

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?
Fixes #93 . Problem was the project name was not being picked correctly.

@CLAassistant
Copy link

CLAassistant commented Sep 1, 2019

CLA assistant check
All committers have signed the CLA.

@sfat sfat force-pushed the feat/get-correct-project-names branch from 07cf673 to 21739bd Compare September 1, 2019 18:43
lib/index.ts Outdated Show resolved Hide resolved
test/system/plugin.test.ts Outdated Show resolved Hide resolved
@sfat sfat changed the base branch from feat/get-correct-project-names to master September 2, 2019 10:46
@sfat sfat requested a review from lili2311 September 2, 2019 12:10
@sfat sfat force-pushed the feat/get-correct-project-names branch from 21739bd to d65f7cf Compare September 2, 2019 12:54
@sfat sfat requested a review from a team as a code owner September 2, 2019 12:54
.gitignore Outdated Show resolved Hide resolved
@sfat sfat force-pushed the feat/get-correct-project-names branch from 0d67bf1 to 9a00db8 Compare September 2, 2019 13:28
@lili2311
Copy link
Contributor

lili2311 commented Sep 2, 2019

@sfat before you merge or release new CLI, please could you verify the project works as expected by doing:

  • snyk monitor using latest CLI version
  • then again snyk monitor using the lasted CLI build locally (to build locally: npm link inside this repo & npm link snyk-gradle-plugin inside snyk/snyk repo and then npm run build in snyk/snyk repo. And verify the same project gets updated and verify what changed in UI if anything visually.

@sfat
Copy link
Contributor Author

sfat commented Sep 2, 2019

@lili2311 I was wondering how to do that.
I'll do that, thank you!

@sfat sfat force-pushed the feat/get-correct-project-names branch from 3b981b8 to 1ad6feb Compare September 5, 2019 19:25
@sfat
Copy link
Contributor Author

sfat commented Sep 5, 2019

I've extended DepTree from snyk-cli-interface to include a new field called newName and made it an optional field.
DepTree seems a good place as it used both for single project, but also for multi project like projects.
Let me know if I'm going in the right direction.

@sfat sfat force-pushed the feat/get-correct-project-names branch from 1ad6feb to 8eb630b Compare September 6, 2019 07:59
@sfat
Copy link
Contributor Author

sfat commented Sep 6, 2019

I've extended DepTree from snyk-cli-interface to include a new field called newName and made it an optional field.
DepTree seems a good place as it used both for single project, but also for multi project like projects.
Let me know if I'm going in the right direction.

I've talked yesterday with @miiila regarding the DepTree and he suggested to add the new name field in the ScannedProject.meta.

This will contain an array (maybe needed for multi level projects where you would have multiple names) of type map, where the key of the map is the old name and the value is the new name.

[
   ["multi-project-parallel","root-proj"],
   ["multi-project-parallel/subproj0","root-proj/subproj0"],
   ["multi-project-parallel/subproj1","root-proj/subproj1"],
   ["multi-project-parallel/subproj2","root-proj/subproj2"],
   ["multi-project-parallel/subproj3","root-proj/subproj3"],
   ["multi-project-parallel/subproj4","root-proj/subproj4"],
]

The reason for this change is that @miiila was saying that DepTree isn't an appropriate data structure for the old/new name convention and is more of a meta information that will be used elsewhere.

Again, let me know if this is okay or you have other ideas.

@sfat
Copy link
Contributor Author

sfat commented Sep 6, 2019

related this this one: snyk/snyk-cli-interface#20

@sfat sfat force-pushed the feat/get-correct-project-names branch 2 times, most recently from e3cd16f to 2796543 Compare September 8, 2019 21:08
@sfat sfat force-pushed the feat/get-correct-project-names branch 3 times, most recently from a0fe7ce to fc122e4 Compare September 16, 2019 11:29
lib/index.ts Outdated Show resolved Hide resolved
lib/index.ts Outdated Show resolved Hide resolved
lib/index.ts Outdated Show resolved Hide resolved
lib/index.ts Outdated Show resolved Hide resolved
lib/index.ts Outdated Show resolved Hide resolved
@sfat sfat force-pushed the feat/get-correct-project-names branch from fc122e4 to b658f37 Compare September 16, 2019 17:02
@sfat sfat requested a review from lili2311 September 16, 2019 17:34
Copy link
Contributor

@lili2311 lili2311 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 nice!

@sfat sfat merged commit c31c28d into snyk:master Sep 17, 2019
@sfat sfat deleted the feat/get-correct-project-names branch September 17, 2019 18:13
@snyksec
Copy link

snyksec commented Sep 17, 2019

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sfat sfat removed the request for review from miiila October 29, 2019 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extract gradle project & sub-project names on every test
5 participants