Move certifications to common #215
Conversation
Current coverage is 92.74% (diff: 98.64%)@@ master #215 diff @@
==========================================
Files 29 31 +2
Lines 903 937 +34
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 824 869 +45
+ Misses 65 55 -10
+ Partials 14 13 -1
|
|
@afeld This should be ready |
| familySummaryMap[newFamily] += exportLink(controlFullName, componentLink) | ||
| summary += "\t" + exportLink(controlFullName, filepath.Join("standards", componentLink)) | ||
| }) | ||
| } | ||
| return summary, &familySummaryMap | ||
| } |
There was a problem hiding this comment.
This is a pretty large function...can we split it up so it's easier to follow?
There was a problem hiding this comment.
it's hard to break up this logic completely. i added some wrappers to some repeated logic to DRY it up and added comments.
There was a problem hiding this comment.
How about making the body of the inner for loop a buildControlSummary() function?
|
Just one thing—looks good otherwise! |
|
Also solves #224 |
| // if there is more left, append the hyphen | ||
| name = fmt.Sprintf("%s%s-", name, fileNamePart) | ||
| } | ||
| } |
There was a problem hiding this comment.
Isn't this what strings.Join() does?
| @@ -0,0 +1,65 @@ | |||
| package certifications | |||
There was a problem hiding this comment.
This doesn't seem to need any unexported(?) identifiers...can we make it the certifications_test package, for better separation?
| @@ -0,0 +1,55 @@ | |||
| package certification | |||
There was a problem hiding this comment.
Ditt about the certification_test package.
Add interface for certification to common.
Addresses https://github.com/opencontrol/compliance-masonry/pull/212/files#r74363210 by using
GetStandardinstead ofGetStandardsA future PR will include the mock for it once the
commonpackage is cleaned up as talked about in #213