0llirocks / cvss-suite

CvssSuite - This Ruby gem helps you to process the vector of the Common Vulnerability Scoring System.
https://cvss-suite.0lli.rocks
Other
23 stars 15 forks source link

Add CvssSuite module on all classes #16

Closed fwininger closed 4 years ago

fwininger commented 4 years ago

Proposed changes

Resolve #11

Types of changes

fwininger commented 4 years ago

You can use the option "hide space change" to simplify the review :

https://github.com/siemens/cvss-suite/pull/16/files?diff=split&w=1

0llirocks commented 4 years ago

I just tested it and it looks good. This change isn't a breaking change as long as people only use CvssSuite and not e.g. Cvss31 directly right?

Still works: cvss = CvssSuite.new('string')

Won't work anymore (without any code change): cvss = Cvss31.new('string', 'version') This would need to be CvssSuite::Cvss31.new('string', 'version') to work. Or you could include the whole namespace.

If you agree I'll put it in the changelog like this.

fwininger commented 4 years ago

This is exactly it. I did not note this as a breaking change because in all the examples include in the readme we only use the CvssSuite method.

fwininger commented 4 years ago

Won't work anymore (without any code change): cvss = Cvss31.new('string', 'version')

I'dont understand why we need to have a 'version' parameter ?

I think that Cvss31 class can implement a method :

def version
  "3.1"
end

I'm surprised we can do :

cvss = Cvss31.new('CVSS:3.1/AV:P/AC:H/PR:L/UI:R/S:U/C:L/I:L/A:H/E:H/RL:U/RC:U', 'hello-kitty)
valid = cvss.valid?   # true
cvss.version          # hello-kitty
0llirocks commented 4 years ago

Good point, will merge your PR and add the change regarding the version method