Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial version of runtimetest command: #9

Closed
wants to merge 6 commits into from

Conversation

zenlint
Copy link
Contributor

@zenlint zenlint commented Jan 21, 2016

Add initial version of runtimetest command:
add unit and config for managing cases;
replace runtime_test.sh to golang realization;

Signed-off-by: zen Lin(Lin Zhinan) [email protected]

@mrunalp
Copy link
Contributor

mrunalp commented Jan 26, 2016

Needs rebase. (On my review list).

@zenlint zenlint force-pushed the initruntimetestCmd branch 5 times, most recently from acb0c2e to 5290838 Compare January 29, 2016 06:55
@zenlint
Copy link
Contributor Author

zenlint commented Jan 29, 2016

@mrunalp
Thanks, I have rebased it, and this is a initial version of runtimetest sub command, let us talk about 'add more test cases and support other runtimes if they have wrappers to CLI similar to runc.
' in the issuse Initial code of runtimetest command #10 for more progress.

@zenlint
Copy link
Contributor Author

zenlint commented Jan 30, 2016

@mrunalp Any review comments plz let me know, thx.

@zenlint zenlint reopened this Mar 24, 2016
Signed-off-by: linzhinan(zen Lin) <[email protected]>
@zenlint
Copy link
Contributor Author

zenlint commented Mar 24, 2016

@liangchenye I have already reabased the PR.

count := 0

for {

Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary empty line here.

@zenlint
Copy link
Contributor Author

zenlint commented Mar 31, 2016

@mrunalp Thanks very much, I have reviewed it myself and also fixed the format problems you pointed out.

Signed-off-by: linzhinan(zen Lin) <[email protected]>
@liangchenye
Copy link
Member

Looks good, I have three suggestions:

  1. less new directory (config and units)
    maybe merge ocitools/config/config.go and ocitools/units/units.go into ocitools/runtimetest.go
  2. fix typos
    although my English grammar is pool, I try to comment on new commits.
  3. less commits
    merge some commits together?

wking referenced this pull request May 9, 2016
wking added a commit to wking/ocitools-v2 that referenced this pull request Feb 10, 2017
These slipped through e41ffc1 (Use RFC 2119's keywords (MUST, MAY,
...), 2015-12-02, opencontainers#9).

Signed-off-by: W. Trevor King <[email protected]>
@wking wking mentioned this pull request Mar 17, 2017
@wking
Copy link
Contributor

wking commented Mar 22, 2017

Close now that #336 has landed?

@Mashimiao
Copy link

as #336 landed, close this

@Mashimiao Mashimiao closed this Apr 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants