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

check for extension and filename first, without load a file content #67

Merged
merged 1 commit into from Jul 10, 2017

Conversation

mcarmonaa
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jul 6, 2017

Codecov Report

Merging #67 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #67   +/-   ##
=======================================
  Coverage   85.03%   85.03%           
=======================================
  Files          16       16           
  Lines         942      942           
=======================================
  Hits          801      801           
  Misses         83       83           
  Partials       58       58

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 d3f79d9...761029c. Read the comment docs.

cli/enry/main.go Outdated
return nil
var language string
var ok bool
if language, ok = enry.GetLanguageByExtension(path); !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

var language string
var ok bool
if language, ok = enry.

could just be replaced by

if language, ok := enry.

or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need the language variable out of the if scope to append it then to the out[] map

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see. You can still leave the ok out then :)

Copy link
Contributor

@abeaumont abeaumont Jul 6, 2017

Choose a reason for hiding this comment

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

I mean something like

var language string
if language, ok := enry...

which would make the need for language after the if statement explicit while making clear that ok is only needed inside the if statement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I can't, if I don't define the ok var out the if too, the := in the if statement would define two new variables https://play.golang.org/p/KWI_sbLOaJ

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see :S. Then it makes more sense to just define them outside the if statement:

language, ok := enry..
if !ok {
}

Anyway, I'm quite happy with it as it is, just trying to understand go better. Thanks for the explanation!

removed language and ok variables declaration
@smola smola merged commit 0582d61 into src-d:master Jul 10, 2017
@mcarmonaa mcarmonaa deleted the fix/cli-optimization branch October 11, 2017 14:51
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.

None yet

3 participants