Skip to content
This repository was archived by the owner on Aug 20, 2021. It is now read-only.

Add oscalkit-test - #31

Merged
anweiss merged 1 commit into
docker-archive:masterfrom
farhankamalkhan-10p:oscalkit-test
Jan 3, 2019
Merged

Add oscalkit-test#31
anweiss merged 1 commit into
docker-archive:masterfrom
farhankamalkhan-10p:oscalkit-test

Conversation

@farhankamalkhan-10p

@farhankamalkhan-10p farhankamalkhan-10p commented Jan 1, 2019

Copy link
Copy Markdown
Contributor

Signed-off-by: farhankamalkhan farhankamalkhan@gmail.com

mohuk
mohuk reviewed Jan 1, 2019
View reviewed changes
Comment thread oscalkit-test/methods.go Outdated
Comment thread oscalkit-test/methods.go Outdated
Comment thread oscalkit-test/methods.go Outdated
Comment thread oscalkit-test/methods.go Outdated
anweiss
anweiss suggested changes Jan 2, 2019
View reviewed changes

@anweiss anweiss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of a directory called oscalkit-test, let's simply call it test.

anweiss
anweiss reviewed Jan 2, 2019
View reviewed changes
Comment thread oscalkit-test/TestCases.go Outdated
anweiss
anweiss reviewed Jan 2, 2019
View reviewed changes
Comment thread oscalkit-test/TestCases.go Outdated
anweiss
anweiss reviewed Jan 2, 2019
View reviewed changes
Comment thread oscalkit-test/TestCases.go Outdated
anweiss
anweiss reviewed Jan 2, 2019
View reviewed changes
Comment thread oscalkit-test/methods.go Outdated
@farhankamalkhan-10p

farhankamalkhan-10p commented Jan 2, 2019

Copy link
Copy Markdown
Contributor Author

@anweiss @mohuk The changes requested have been made.

@farhankamalkhan-10p

farhankamalkhan-10p commented Jan 2, 2019

Copy link
Copy Markdown
Contributor Author

@anweiss Just as a clarification, we are using github.com/fatih/color to display the test result in green or red in case the test passes or fails respectively, currently the user will have to go get the dependency and this is mentioned in the readme file. Is this approach correct or should we add it to vendor?

@anweiss

anweiss commented Jan 2, 2019

Copy link
Copy Markdown
Contributor

@farhan-khan30 please vendor the deps

@anweiss

anweiss commented Jan 2, 2019

Copy link
Copy Markdown
Contributor

@farhan-khan30 also bear in mind that https://github.com/fatih/color was recently archived and is no longer maintained.

@anweiss anweiss added the test Unit, functional and end-to-end testing label Jan 2, 2019
@anweiss anweiss added this to the OSCAL 1.0 milestone Jan 2, 2019
anweiss
anweiss previously approved these changes Jan 2, 2019
View reviewed changes
@mohuk

mohuk commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

Although https://github.com/fatih/color is archived, but it looks to be in fairly good shape. We can continue with this for now. If it breaks on any major update in Go, we'd fork, fix whats required and switch to our fork in the deps.

@farhankamalkhan
Add oscalkit-test
942d806 
Signed-off-by: farhankamalkhan <farhankamalkhan@gmail.com>
@farhankamalkhan-10p

farhankamalkhan-10p commented Jan 3, 2019

Copy link
Copy Markdown
Contributor Author

@anweiss added the deps to vendor and updated the readme.

@anweiss

anweiss commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

LGTM.

anweiss
anweiss approved these changes Jan 3, 2019
View reviewed changes

@anweiss anweiss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@anweiss
anweiss merged commit dc65f31 into docker-archive:master Jan 3, 2019
asadullah-yousuf-10p pushed a commit to asadullah-yousuf-10p/oscalkit that referenced this pull request Jan 9, 2019
@farhankamalkhan-10p @asadullah-yousuf-10p
Add oscalkit-test (docker-archive#31)
ee7ab1d 
Signed-off-by: farhankamalkhan <farhankamalkhan@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@mohuk mohuk mohuk left review comments
@anweiss anweiss anweiss approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

test Unit, functional and end-to-end testing

Projects

None yet

Milestone

OSCAL 1.0

Development

Successfully merging this pull request may close these issues.

4 participants

@farhankamalkhan-10p @anweiss @mohuk @farhankamalkhan