Refactor opencontrol interface and clean common part 1. #217
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Current coverage is 92.03% (diff: 94.32%)
@@ add-result-package #217 diff @@
====================================================
Files 32 30 -2
Lines 968 979 +11
Methods 0 0
Messages 0 0
Branches 0 0
====================================================
+ Hits 888 901 +13
+ Misses 65 64 -1
+ Partials 15 14 -1
|
jcscottiii
changed the title
jcscottiii
force-pushed
the
move-opencontrol-interface
branch
2 times, most recently
from
419e4cb
to
ecd0580
Compare
| }) | ||
|
|
||
|
|
||
| var _ = Describe("Entry", func() { |
There was a problem hiding this comment.
/me is assuming that's a TODO for yourself...?
| table.DescribeTable("DownloadRepo", func(err error) { | ||
| remoteSource := new(commonMocks.RemoteSource) | ||
| remoteSource.On("GetURL").Return("") | ||
| remoteSource.On("GetRevision").Return("") |
There was a problem hiding this comment.
these don't matter as much since it's not actually downloading something. but i can put something here so people know what to expect!
|
Phew, ok, a handful of notes. Let's please please please try and keep the pull requests smaller for reviewer sanity 😁 |
jcscottiii
force-pushed
the
add-result-package
branch
from
4bee222
to
13cd623
Compare
jcscottiii
force-pushed
the
move-opencontrol-interface
branch
from
ecd0580
to
92d4c55
Compare
|
@afeld this should be ready! |
jcscottiii
force-pushed
the
add-result-package
branch
from
13cd623
to
f898b26
Compare
jcscottiii
force-pushed
the
move-opencontrol-interface
branch
from
47f6fd6
to
9cbd029
Compare
added 8 commits
October 3, 2016 14:50
This was neccessary to clean up `common` package
added 5 commits
October 3, 2016 14:50
We can dot import table package because we no longer have a struct called Entry (now it's called VCSEntry)
afeld
approved these changes
Oct 3, 2016
| "github.com/opencontrol/compliance-masonry/tools/mapset" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/mock" | ||
| ) |
There was a problem hiding this comment.
This many imports seems like a red flag that the setup for these structs/functions are too complicated, though I don't have specific suggestions about how to simplify 😕
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds on #216
This was necessary to clean up
lib/commonpackage.The
lib/commonpackage had both interfaces and structs for the YAMLs.This led to weird import paths to get around circular dependencies.
This patch is just mostly moving stuff around.
EntrytoRemoteSourcein thelib/commonpackageEntrywas. It really was a container of information to some remote (re)sourceRemoteSourcefor the v1.0.0 of theOpenControlis now calledVCSEntry(Probably should be calledVCSSource)Entry/VCSEntry(with the yaml tags) for v1.0.0 was in thelib/commonpackage. Not where things should be for implementation. Hence, it has moved tolib/opencontrol/versions/1.0.0/opencontrol.goEntryDownloaderwas renamed toDownloaderEntryno longer exists due to the last point.lib/opencontrolandcommands/get.libpackage. (GetResources,GetRemoteResources,GetLocalResources). This made it very hard to separate outlib/commoncommands/get/resourcespackagelib/opencontrol/versions/basepackageOpenControlfromlib/opencontrol/versions/base/tolib/commonParse,GetSchemaVersionandGetResourcestoGetCertificationsGetStandardsGetComponentsGetCertificationsDependenciesGetStandardsDependenciesGetComponentsDependencieslibandcommand(which is also nice for plugins)Workerstruct. It created a circular dependency when trying to use with mocks.