Super Duper Mortal Mario Kombat: Plug all the leaks #141

Closed
nulltoken opened this Issue Apr 29, 2012 · 9 comments

Comments

Projects
None yet
3 participants
Member

nulltoken commented Apr 29, 2012

Life might be less boring. Let's play!

Rules

Set up the board game

  • Get some inspiration from the comments of commit dahlbyk@d430855
  • Create a dedicated build configuration. Give it a fancy explicit name.
  • Patch SafeHandleBase. Don't get hurt. This one is nasty....
  • Commit. A nice commit message adds bonus points

Enter the arena

  • Fight those leaks. One commit at a time. Don't drown
  • Nice refactorings and code removal grant style points

Claim your prize

  • Send a nice PR
  • Update the Wiki Contributing section
  • Profit ! And let everyone bathe in your awesomeness :)
Contributor

aroben commented Apr 30, 2012

❤️ this idea.

Member

dahlbyk commented May 22, 2012

Noticed dahlbyk@d430855 had disappeared so I found the commit locally and pushed it back up...

Member

nulltoken commented May 22, 2012

🎰🎰🎰🎰🎰🎰

@dahlbyk Wow...you're scoring!

Member

dahlbyk commented May 23, 2012

Ignoring invalid handles, I think this does it...

Meta: am I missing a way to associate my branch with this issue, short of tagging the issue in a commit message?

Member

nulltoken commented May 23, 2012

@dahlbyk Secret combination a keystrokes... and voila! Fatality!

Looks like you cleaned them all.

achievement

One last concern, the building of the stacktrace is slowing down the test run a bit.

#if DAHLBYK_RULZ:
415 passed, 0 failed, 1 skipped (see 'Task List'), took 29,63 seconds (xUnit.net 1.9.0 build 1566).

#if DEBUG:
415 passed, 0 failed, 1 skipped (see 'Task List'), took 62,17 seconds (xUnit.net 1.9.0 build 1566).

Could you create a dedicated build configuration?

I added two more nitpicks in the comments of the commits ;)

Meta: am I missing a way to associate my branch with this issue, short of tagging the issue in a commit message?

I don't know of any easy way to do this. There are backstage ones, however...

@dahlbyk dahlbyk added a commit to dahlbyk/libgit2sharp that referenced this issue May 23, 2012

@dahlbyk @dahlbyk dahlbyk + dahlbyk Add Leaks build configuration for #141 65aa0aa
Member

dahlbyk commented May 23, 2012

Cleaned up commits have been pushed to https://github.com/dahlbyk/libgit2sharp/tree/issue141

I left warnings in for DEBUG and only dump the stack trace for a LEAKS configuration - how does performance look for each? On my machine there's not much difference, which seems curious.

I don't know of any easy way to do this. There are backstage ones, however...

Neat

Member

nulltoken commented May 23, 2012

Sorry. It took some time, but the CI server was on strike.

The branch has been merged.

Awesome job! Really.

nulltoken closed this May 23, 2012

Member

nulltoken commented May 29, 2012

Released as part of v0.9.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment