-
-
Notifications
You must be signed in to change notification settings - Fork 667
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
Make output modes configurable #60
Conversation
yugui
commented
Aug 3, 2016
•
edited
Loading
edited
- Extracted from Import Gazel, BUILD file generator #51
- Depends on Import minimal version of gazelle, a BUILD file generator #57, Support dependencies to go_library rules in the same repository #58 and Support more rules in Gazel #59
- c.f. Create "glaze" BUILD file generator for Go #15
6907367
to
7ae0cfe
Compare
if err != nil { | ||
return err | ||
} | ||
defer os.Remove(f.Name()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my experience, worrying about the error from the defer is overkill, and creates complexity without much gain.
I agree.
Actually in my experience it was necessary to correctly deal with disk full in the target file system, temporary network issue or something. But as you said, worrying about that does not add much value in this specific case of short-living process unlike my experience.
I would have written (instead of line 31-49):
defer os.Remove(f.Name()) defer f.Close() if _, err := f.Write(bzl.Format(file)); err != nil { return err }
Thank you for your suggestion. Fixed.
But I added Sync
because Close
used to also imply Sync
in the original code.
@pmbethe09 Could you take another look at this PR? |
} | ||
defer os.Remove(f.Name()) | ||
defer f.Close() | ||
if _, err = f.Write(bzl.Format(file)); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use := instead of = whenever possible, like here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
LGTM with a couple of nitpicks. |
Added "fix" and "diff" modes
Thank you for your review. |