Fix memory leaks

Authored by abalaji on May 2 2018, 7:56 PM.



Fix a few memory leaks, and some other minor fixes


Diff Detail

R166 Spectacle
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
abalaji requested review of this revision.May 2 2018, 7:56 PM
abalaji created this revision.
abalaji edited the summary of this revision. (Show Details)May 2 2018, 7:59 PM
abalaji added a project: Spectacle.

Split off from D12626

rkflx added a comment.May 3 2018, 12:24 PM

Thanks for fixing this ;)

LGTM (mostly).

35 ↗(On Diff #33513)

I know that sometimes there is the urge to fix "just another small thing". However, in general we prefer to stick to one topic/change per Diff, in order to keep the Git history somewhat clean.

If you find it important enough to remove the include and demand your reviewer's time, send another Diff, otherwise just ignore it. We are aware that Spectacle's code is a bit messy in some parts.

I admit it's very nitpicky, but please remove this change.

abalaji updated this revision to Diff 33605.May 4 2018, 7:29 AM
  • Undo change
abalaji marked an inline comment as done.May 4 2018, 7:30 AM


rkflx accepted this revision.May 4 2018, 11:22 AM
This revision is now accepted and ready to land.May 4 2018, 11:22 AM
Closed by commit R166:d929ba42d01d: Fix memory leaks (authored by abalaji, committed by rkflx). · Explain WhyMay 4 2018, 11:25 AM
This revision was automatically updated to reflect the committed changes.
rkflx added a comment.May 4 2018, 11:28 AM

@abalaji Congratulations, your first two patches are in. I guess the next ones won't be that easy ;)

Hooray, bless! You say the next ones won't be this easy, I believe you haha