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 virtletctl version #658

Merged
merged 2 commits into from
Apr 18, 2018
Merged

Add virtletctl version #658

merged 2 commits into from
Apr 18, 2018

Conversation

ivan4th
Copy link
Contributor

@ivan4th ivan4th commented Apr 17, 2018

  • implement virtletctl version
  • make sure it works with Virtlet release mechanism

This change is Reviewable

@ivan4th ivan4th changed the title [WiP] Add virtletctl version Add virtletctl version Apr 17, 2018
@pigmej
Copy link
Contributor

pigmej commented Apr 17, 2018

:lgtm:


Reviewed 38 of 38 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@jellonek
Copy link
Contributor

Reviewed 9 of 38 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


cmd/virtlet/virtlet.go, line 180 at r1 (raw file):

	flag.Parse()
	if *displayVersion {
		printVersion()

Imo os.Exit should be called from main, not from above func, but I'm fine with that also...


pkg/tools/version.go, line 91 at r1 (raw file):

			continue
		case exitCode != 0:
			errors = append(errors, fmt.Sprintf("node %q: error getting version from Virtlet pod %q: %v", nodeName, podName, err))

There should be probably shown exitCode as err is equal in this case to nil.
If it should have the same body it should be probably denoted as case err != nil, exitCode != 0:


Comments from Reviewable

@ivan4th
Copy link
Contributor Author

ivan4th commented Apr 18, 2018

Review status: 36 of 38 files reviewed at latest revision, 2 unresolved discussions.


cmd/virtlet/virtlet.go, line 180 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Imo os.Exit should be called from main, not from above func, but I'm fine with that also...

Done.


pkg/tools/version.go, line 91 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

There should be probably shown exitCode as err is equal in this case to nil.
If it should have the same body it should be probably denoted as case err != nil, exitCode != 0:

Done.


Comments from Reviewable

@jellonek
Copy link
Contributor

:lgtm:


Reviewed 1 of 2 files at r2.
Review status: 37 of 38 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@pigmej
Copy link
Contributor

pigmej commented Apr 18, 2018

:lgtm:


Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@pigmej pigmej merged commit 6740d5c into master Apr 18, 2018
@pigmej pigmej deleted the ivan4th/version branch April 18, 2018 08:59
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.

3 participants