Elixir Security Importer - #294
Conversation
steven-esser
left a comment
There was a problem hiding this comment.
@tushar912 Just a few formatting and stylistic comments.
Also, please squash your minor commits into a single and write a descriptive commit message.
This helps future developers understand what exactly was added or changed when looking at the history.
Thanks for your contribution!
|
Thanks ! @tushar912 I've done a light review for now, see comments inline. I'll do another review once this is resolved. |
Signed-off-by: Tushar912 <tushar.912u@gmail.com> add elixir security to init.py Signed-off-by: Tushar912 <tushar.912u@gmail.com> add test for elixir security Signed-off-by: Tushar912 <tushar.912u@gmail.com> fixed code style Signed-off-by: Tushar912 <tushar.912u@gmail.com>
Signed-off-by: Tushar912 <tushar.912u@gmail.com>
| version_list.append(release["version"]) | ||
| return version_list | ||
|
|
||
| def get_pkg_from_range(self, version_list, pkg_name): |
There was a problem hiding this comment.
The function says get_pkg_from_range but returns a version list
There was a problem hiding this comment.
is get_versions_from_range better
Signed-off-by: Tushar912 <tushar.912u@gmail.com>
Signed-off-by: Tushar912 <tushar.912u@gmail.com>
|
@tushar912 btw after the rebase :
|
|
@sbs2001 should i fetch the latest changes and resolve the conflicts |
|
I think you have added a |
Yes !
That will be great use it :) . |
Signed-off-by: Tushar912 <tushar.912u@gmail.com>
sbs2001
left a comment
There was a problem hiding this comment.
Thanks @tushar912 for your patience, we're almost done see my comments inline
|
@tushar912 what is b0e9451 ? |
used load_yaml from helpers Signed-off-by: Tushar912 <tushar.912u@gmail.com> fix typo in importer Signed-off-by: Tushar912 <tushar.912u@gmail.com>
|
@sbs2001 i have reworded that |
Signed-off-by: Tushar912 <tushar.912u@gmail.com>
|
|
||
| return packages | ||
|
|
||
| def get_versions_from_range(self, version_list, pkg_name): |
There was a problem hiding this comment.
A comment or docstring would be very helpful here.
Actually this function doesn't make sense it's called get_versions_from_range but the parameters don't have suggest any sort of range
There was a problem hiding this comment.
Plus the name of package is required, which the name of the function doesn't suggest
There was a problem hiding this comment.
should I rename it as get_versions_for_pkg_from_range_list. I would rename the parameter version_list to version_range_list. I would make sure to add comment also
There was a problem hiding this comment.
That's verbose, but fine by me :) . A better design would be to decouple the obtaining of all versions of package from categorising the versions.
Signed-off-by: Tushar912 <tushar.912u@gmail.com>
| from packageurl import PackageURL | ||
|
|
||
| from vulnerabilities.data_source import GitDataSource | ||
| from vulnerabilities.data_source import GitDataSourceConfiguration |
There was a problem hiding this comment.
This import is not required
| yaml_file["unaffected_versions"] = [] | ||
|
|
||
| safe_pkg_versions, vuln_pkg_versions = self.get_versions_for_pkg_from_range_list( | ||
| yaml_file.get("patched_versions") + yaml_file.get("unaffected_versions"), |
sbs2001
left a comment
There was a problem hiding this comment.
@tushar912 This PR can be merged after fixing the few nits I've suggested.
Also add your name in the CONTRIBUTING.rst.
|
@sbs2001 what should I write in ecosystems covered in |
|
@tushar912 the ecosystem would be |
|
@sbs2001 I have made the changes |
| +----------------+------------------------------------------------------------------------------------------------------+----------------------------------------------------+ | ||
| |postgresql | https://www.postgresql.org/support/security/ |postgresql | | ||
| +----------------+------------------------------------------------------------------------------------------------------+----------------------------------------------------+ | ||
| |elixir_security | https://github.com/dependabot/elixir-security-advisories' |hex packages | |
There was a problem hiding this comment.
Remove the single quote at end of the url
There was a problem hiding this comment.
Oh Sorry I missed it.I will make the change
Signed-off-by: Tushar912 <tushar.912u@gmail.com> remove single quote at end of url Signed-off-by: Tushar912 <tushar.912u@gmail.com>
sbs2001
left a comment
There was a problem hiding this comment.
LGTM ! Thanks @tushar912 I'm merging this :)
Fixes #287 by @sbs2001
Description
I have added importer and tests for elixir security