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.

Sign up for GitHub

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

Jump to bottom

fix #314: --version not showing lab version #346

Merged
merged 1 commit into from
Nov 27, 2019
Merged

fix #314: --version not showing lab version #346

merged 1 commit into from
Nov 27, 2019

Conversation

zaquestion
Copy link
Owner

No description provided.

@codecov
Copy link

codecov bot commented Oct 25, 2019 •
edited
Loading

Codecov Report

Merging #346 into master will increase coverage by 0.16%.
The diff coverage is 55.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #346      +/-   ##
==========================================
+ Coverage   64.08%   64.25%   +0.16%     
==========================================
  Files          49       49              
  Lines        2595     2596       +1     
==========================================
+ Hits         1663     1668       +5     
+ Misses        804      800       -4     
  Partials      128      128
Impacted Files Coverage Δ
cmd/snippet_create.go 83.33% <100%> (+2.22%) ⬆️
cmd/project_create.go 90.9% <100%> (+4.54%) ⬆️
internal/git/git.go 68.9% <100%> (+0.26%) ⬆️
cmd/root.go 71.01% <20%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d80422...284fbb8. Read the comment docs.

claytonrcarter
claytonrcarter reviewed Oct 25, 2019
View reviewed changes
cmd/project_create.go Outdated
@@ -66,6 +67,7 @@ lab project create -n "new proj" # user/new-proj named "new proj"`,
if err != nil {
log.Fatal(err)
}
spew.Dump(p)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@zaquestion did you mean to leave this in? My Go is rusty, but this looks like a debugging statement.

@claytonrcarter
Copy link
Collaborator

Aside from the [possible] debugging statement, the only other comment I had is that changing the default visibility from "internal" to "private" -- though needed -- seemed beyond the scope of a PR that's supposed to be fixing --version and I just want to confirm that you intended it to be in here. Sorry if I'm being nitpicky.

@zaquestion
Copy link
Owner Author

All in an effort to fix the tests, I can split it out, I'm just glad to see it seemed to work

@claytonrcarter
Copy link
Collaborator

All good! Just double checking that you meant it all to be there.

@zaquestion zaquestion mentioned this pull request Nov 27, 2019
Merged
@zaquestion zaquestion merged commit 9252eb2 into master Nov 27, 2019
@zaquestion zaquestion added this to the 0.17.0 milestone Dec 11, 2019
@zaquestion zaquestion deleted the fix_version_flag branch December 16, 2019 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claytonrcarter claytonrcarter claytonrcarter left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.17.0
Development

Successfully merging this pull request may close these issues.
2 participants
@zaquestion @claytonrcarter