Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #6340 (reopened enhancement)

Opened 2 years ago

Last modified 10 months ago

[PATCH] [continuous_builder] trivial improvements

Reported by: tec Assigned to: David
Priority: normal Milestone: 2.x
Component: Plugins Version:
Severity: normal Keywords: plugin continuous_builder
Cc:

Attachments

continuous_builder.patch (0.6 kB) - added by tec on 10/03/06 10:02:35.

Change History

10/03/06 10:02:35 changed by tec

  • attachment continuous_builder.patch added.

02/24/07 19:02:24 changed by josh

  • status changed from new to closed.
  • resolution set to untested.
  • component changed from Web to Plugins.

07/19/07 13:42:21 changed by mpalmer

The problem I see with trying to test this is that the patch doesn't actually change functionality -- it's just a code improvement. Well, the first change could perhaps be tested by running the tests on windows, but the second just looks like a straight-up code improvement.

Does anyone with a better knowledge of the patch triaging procedures wish to comment on what the procedure should be for this patch?

07/19/07 22:26:34 changed by mpalmer

  • status changed from closed to reopened.
  • resolution deleted.

There are no possible test cases that I can think of to write for this patch, but it's a solid improvement in code quality as far as I'm concerned. +1

(follow-up: ↓ 6 ) 07/20/07 06:35:27 changed by manfred

  • status changed from reopened to closed.
  • resolution set to incomplete.

Tec, can you add a description to your patch so the rest of us knows what problem this patch solves?

After that we can discuss whether or not it needs tests.

07/20/07 07:41:57 changed by mpalmer

  • status changed from closed to reopened.
  • resolution deleted.

C'mon Manfred, the patch is pretty obvious:

  • use File.join instead of adding strings together, so that we're file-separator-independent;
  • Use release_lock instead of deleting the lock file directly, to hide implementation from interface.

(in reply to: ↑ 4 ) 07/20/07 07:53:08 changed by tec

Replying to manfred:

Tec, can you add a description to your patch so the rest of us knows what problem this patch solves?

It doesn't really solve any problems - I wrote the patch because I thought the code could be written slightly better. IMO it's an improvement, but I appreciate that's very subjective, so don't really mind whether it's applied or not.

07/20/07 08:01:23 changed by manfred

I don't want to start a whole discussion about this, but I think every ticket should at least have a short description. I'm willing to discuss this further on IRC>

Anyway, the code looks at lot better like this. +1 for applying.