Rails core - Jul 2007 - Patch Process

Hey guys,

In the interest of streamlining the patch-submission process, we''ve
been thinking about writing down the informal vetting process we''ve
been using for patches.  The first cut of this is written down in the
wiki:

http://wiki.rubyonrails.org/rails/pages/PatchRequirements

Any comments or questions?

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Sounds cool.

You can probably mention #rails-contrib as well for finding potential reviewer.

And how about having a rule to close the ticket as invalid/wontfix if
it''s been there for over 2-3 months without any updates ? And may be
get a core member to reopen it if one feels the patch has a chance of
making it to core ever. I was just going through some of the old
tickets, and I feel that very small % of them will ever make it to
core. Probably closing the remaining tickets would be good to keep
things clean on trac as some of them are even 2 years old.

One more thing I always wonder about is if I should assign the ticket
to a core member or just leave it with default assignee "core". You
think it might be a good idea to assign ticket to the particular core
member who has been the most active in the area that patch affects ?
If yes, may be you can put the mapping at that wiki page ? Just a
thought.

Thanks,
Pratik

On 7/14/07, Michael Koziarski <michael@koziarski.com>
wrote:
>
> Hey guys,
>
> In the interest of streamlining the patch-submission process,
we''ve
> been thinking about writing down the informal vetting process
we''ve
> been using for patches.  The first cut of this is written down in the
> wiki:
>
> http://wiki.rubyonrails.org/rails/pages/PatchRequirements
>
> Any comments or questions?
>
> --
> Cheers
>
> Koz
>
> >
>

-- 
http://m.onkey.org

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Requiring peer review is great. Also, multiple peer review can help  
mitigate the tyranny of the zealous :)

I don''t personally have time to make core patches any more but would  
be happy to help the community in this way by moderating. Great ideas!

-------
Courtenay
http://blog.caboo.se

On Jul 13, 2007, at 8:32 PM, "Michael Koziarski"  
<michael@koziarski.com> wrote:
>
> Hey guys,
>
> In the interest of streamlining the patch-submission process,
we''ve
> been thinking about writing down the informal vetting process
we''ve
> been using for patches.  The first cut of this is written down in the
> wiki:
>
> http://wiki.rubyonrails.org/rails/pages/PatchRequirements
>
> Any comments or questions?
>
> -- 
> Cheers
>
> Koz
>
> >
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Jul 13, 8:32 pm, "Michael Koziarski" <mich...@koziarski.com>
wrote:
> Any comments or questions?
Looks about right.

Maybe expand on the warning bitsweat added to the trac home page.

"Expect your ticket to be closed with an untested, undocumented, or
incomplete resolution if it''s missing tests, documentation, or
implementation. Don''t panic; the ticket hasn''t been killed!
These
resolutions are the pathway to commit. Update your patch and reopen
the ticket."


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Fri, Jul 13, 2007 at 10:04:40PM -0700, Josh Peek
wrote:
> Maybe expand on the warning bitsweat added to the trac home page.
> 
> "Expect your ticket to be closed with an untested, undocumented, or
> incomplete resolution if it''s missing tests, documentation, or
> implementation. Don''t panic; the ticket hasn''t been
killed! These
> resolutions are the pathway to commit. Update your patch and reopen
> the ticket."
This seems heavy-handed for patches which fix defects.  I mean, if a defect
report didn''t have a patch at all, it''d stay open, but if
it''s a *better*
report (ie closer to being fixed, just not quite all the way there) then it
gets closed?  That seems... counterproductive.  (Disclaimer: I''m not a
neutral party on this topic -- I''ve had a couple of bugs with patches
treated this way recently; it *really* made me less interested in
contributing future patches).  I understand why patches get triaged, and
there is the possibility of bad patches hanging around forever, but surely
the policy of expiring old bug reports could also apply to half-baked
patches, rather than instant closure?

Also, I presume that this PatchProcess page will be rolled into or otherwise
linked from http://dev.rubyonrails.org once it''s considered acceptable?

- Matt

-- 
"For once, Microsoft wasn''t exaggerating when they named it the
''Jet Engine''
-- your data''s the seagull."
		-- Chris Adams

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> I understand why patches get triaged, and
> there is the possibility of bad patches hanging around forever, but surely
> the policy of expiring old bug reports could also apply to half-baked
> patches, rather than instant closure?
It''s not just the possibility of patches sitting around, we had
hundreds of patches just sitting around and the submitters commented
that the lack of feedback was more frustrating than having a patch
''rejected''.

What was it about the closure that made it irksome?  Tickets get
reopened all the time, closure isn''t a death sentence.  But
it''s more
honest than just having an atrophied patch sitting in the queue.
> Also, I presume that this PatchProcess page will be rolled into or
otherwise
> linked from http://dev.rubyonrails.org once it''s considered
acceptable?
Yep.
> - Matt
>
> --
> "For once, Microsoft wasn''t exaggerating when they named it
the ''Jet Engine''
> -- your data''s the seagull."
>                 -- Chris Adams
>
> >
>

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

So, if I''m the 3rd reviewer on someone''s patch and I leave a
thumbs-up
comment, it''s free for me to add the "verified" keyword?

@Josh Peek: +1 for the "don''t panic" notice. First-time
contributors are
easily discouraged otherwise.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 7/13/07, Matthew Palmer <mpalmer@hezmatt.org> wrote:
> This seems heavy-handed for patches which fix defects.  I mean, if a defect
> report didn''t have a patch at all, it''d stay open, but if
it''s a *better*
> report (ie closer to being fixed, just not quite all the way there) then it
> gets closed?  That seems... counterproductive.  (Disclaimer: I''m
not a
> neutral party on this topic -- I''ve had a couple of bugs with
patches
> treated this way recently; it *really* made me less interested in
> contributing future patches).  I understand why patches get triaged, and
> there is the possibility of bad patches hanging around forever, but surely
> the policy of expiring old bug reports could also apply to half-baked
> patches, rather than instant closure?
Really?  Cuz I had patches that got ignored, so I stopped
contributing.  Having them rejected with a reason would have helped.

That said, I like the new wiki page.  Simple and to the point.  The
patch process becoming more transparent is a real win, thanks guys.

-- 
Chris Wanstrath
http://errfree.com // http://errtheblog.com

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> So, if I''m the 3rd reviewer on someone''s patch and I
leave a thumbs-up
> comment, it''s free for me to add the "verified" keyword?
That''s the intention yeah.
> @Josh Peek: +1 for the "don''t panic" notice. First-time
contributors are
> easily discouraged otherwise.
It''s probably worthwhile coming up with some ''canned''
text to paste in
when taking certain actions.  I''d guess it''s much more
reassuring if
you can see a path to getting your patch applied.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> Really?  Cuz I had patches that got ignored, so I stopped
> contributing.  Having them rejected with a reason would have helped.
That''s the conclusion we''ve come to.  Better to reject
something
quickly, than let it atrophy forever for fear of offending someone.
> That said, I like the new wiki page.  Simple and to the point.  The
> patch process becoming more transparent is a real win, thanks guys.
So far this is just a plan,  hopefully it''ll help out.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> So far this is just a plan,  hopefully it''ll help out.
To clarify, we''re doing this.  Time will tell if it improves the
turnaround time on patches. :)

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Sat, Jul 14, 2007 at 06:07:06PM +1200, Michael Koziarski
wrote:
> > I understand why patches get triaged, and
> > there is the possibility of bad patches hanging around forever, but
surely
> > the policy of expiring old bug reports could also apply to half-baked
> > patches, rather than instant closure?
> 
> It''s not just the possibility of patches sitting around, we had
> hundreds of patches just sitting around and the submitters commented
> that the lack of feedback was more frustrating than having a patch
> ''rejected''.
Yes, lack of feedback sucks, but there are forms of feedback other than
"closed defect: incomplete".
> What was it about the closure that made it irksome?  Tickets get
> reopened all the time, closure isn''t a death sentence.
''Closed'' is one of the fundamental states of a ticket, and it
means
something fairly definite to me -- something along the lines of "absent
some
exceptional condition, this is done with".

The thing that really got to me was that, if I had just reported the bug and
left off the patch, it would have stayed open for someone else to deal with. 
However, having taken the extra time to hunt down a solution and post the
patch (however incomplete it may have been), the ticket was closed and,
absent further interaction from me, it was as though the defect was never
reported.  If the patch is dodgy, then reject the *patch*, not the entire
defect report.  I very nearly opened a completely separate bug report
without a patch just to see what would happen.

What would happen if a bug was reported by one person but an incomplete
patch contributed by someone else?  Would the report get closed (which to
the initial reporter suggests the issue is solved) because the patch is
dodgy?

- Matt

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Sat, Jul 14, 2007 at 01:04:52AM -0700, Chris Wanstrath
wrote:
> On 7/13/07, Matthew Palmer <mpalmer@hezmatt.org> wrote:
> > This seems heavy-handed for patches which fix defects.  I mean, if a
defect
> > report didn''t have a patch at all, it''d stay open,
but if it''s a *better*
> > report (ie closer to being fixed, just not quite all the way there)
then it
> > gets closed?  That seems... counterproductive.  (Disclaimer:
I''m not a
> > neutral party on this topic -- I''ve had a couple of bugs with
patches
> > treated this way recently; it *really* made me less interested in
> > contributing future patches).  I understand why patches get triaged,
and
> > there is the possibility of bad patches hanging around forever, but
surely
> > the policy of expiring old bug reports could also apply to half-baked
> > patches, rather than instant closure?
> 
> Really?  Cuz I had patches that got ignored, so I stopped
> contributing.  Having them rejected with a reason would have helped.
I have nothing against feedback.  Feedback is good.  The only way
you''re going to get better patches over time is if you communicate how
the
patches you''re getting are insufficient.  My *only* gripe is with the
manner
in which the communication is being performed.  I have a particular
definition of what ''closed'' means to me, and it''s not
"OK, time to write a
test case".

- Matt

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 7/14/07, Matthew Palmer <mpalmer@hezmatt.org>
wrote:
> I have a particular
> definition of what ''closed'' means to me, and
it''s not "OK, time to write a
> test case".
>
> - Matt
Does the fact that it''s being said that a closure isn''t a
death
sentence, just a matter of feedback change your opinion of
''closed''?
Do the ''untested'', ''undocumented''
resolutions hurt less?


-- 
Kevin Clark
http://glu.ttono.us

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Jul 14, 3:04 am, "Chris Wanstrath" <ch...@ozmm.org>
wrote:
> Really?  Cuz I had patches that got ignored, so I stopped
> contributing.  Having them rejected with a reason would have helped.
I agree there, because nothing gets peoples attention like closing
their ticket ;)


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Jul 14, 6:13 am, Matthew Palmer <mpal...@hezmatt.org>
wrote:
> What would happen if a bug was reported by one person but an incomplete
> patch contributed by someone else?  Would the report get closed (which to
> the initial reporter suggests the issue is solved) because the patch is
> dodgy?
Usually patches with good units tests that prove a bug are fixed
rather quickly. The problem is we have tons of unverified (and bogus)
defect reports with no easy way to prove them. No one has the time to
personally debug someone elses problems.

We should post a note to try the Mailing List or IRC channel first if
you are unsure about your patch.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Sat, Jul 14, 2007 at 08:30:30AM -0700, Kevin Clark
wrote:
> 
> On 7/14/07, Matthew Palmer <mpalmer@hezmatt.org> wrote:
> > I have a particular
> > definition of what ''closed'' means to me, and
it''s not "OK, time to write a
> > test case".
> >
> > - Matt
> 
> Does the fact that it''s being said that a closure isn''t a
death
> sentence, just a matter of feedback change your opinion of
''closed''?
No, because it''s an emotional response, not a purely intellectual one.
> Do the ''untested'', ''undocumented''
resolutions hurt less?
They''re not "resolutions", they''re states.  You set
them as flags or
something.  The very meaning of word "resolution" implies some sort of
finality, but a defect report that isn''t fixed isn''t resolved.

- Matt

-- 
Skippy was a wallaby. ... Wallabies are dumb and not very trainable...  The
*good* thing...is that one Skippy looks very much like all the rest,
hence..."one-shot Skippy" and "plug-compatible Skippy".  I
don''t think they
ever had to go as far as "belt-fed Skippy" 	-- Robert Sneddon, ASR

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Sat, Jul 14, 2007 at 08:48:20AM -0700, Josh Peek
wrote:
> On Jul 14, 6:13 am, Matthew Palmer <mpal...@hezmatt.org> wrote:
> > What would happen if a bug was reported by one person but an
incomplete
> > patch contributed by someone else?  Would the report get closed (which
to
> > the initial reporter suggests the issue is solved) because the patch
is
> > dodgy?
> 
> Usually patches with good units tests that prove a bug are fixed
> rather quickly.
(Good thing you put ''usually'' in there, because I''ve
got a patch in the
system that fixes a bug in the *test suite* that''s been hanging for
over a
week now)

Is it the policy of the Rails project that defect reports without a
verifiable test case are invalid and will be closed?

If a project was going to adopt that policy, something like Rails would be
the most likely to be able to pull it off, because every user should be a
developer.  Having spent a few hours a day the last few weeks helping on the
#rubyonrails IRC channel, though, I''m not sure it''s a
practical proposition
-- most people don''t "get" testing, and plenty of
Rails'' users aren''t
sufficiently good programmers to be able to write a solid test case for
something as complex as Rails.
> The problem is we have tons of unverified (and bogus)
> defect reports with no easy way to prove them.
So mark them "more info required" with a note that they need to have
more
info attached or they''ll be closed in N days/weeks/months.
> No one has the time to
> personally debug someone elses problems.
<advocate type="devil">
 That cuts both ways, though -- I, as a developer, don''t have time to
debug a
 problem in Rails.  That problem isn''t mine -- I didn''t write
the buggy code
 that''s causing the hassle.  It''s *your* fault, dammit.
</advocate>

In reality, I know all about the OSS project philosophy, I''m a great
believer in it myself.  If you want support, take out a support contract,
and all that.

Going back to my original point, though, about actively *closing* tickets
that document *real* bugs that are trivially verifiable[1] simply because
the patch isn''t sufficient is only reasonable in one circumstance -- if
defect reports without test cases are summarily closed.  I haven''t seen
that
policy documented anywhere (and I would imagine something that it would want
to be in big letters on the front page), and in fact the exact opposite is
stated on the dev front page -- "Tickets are fine".  I''m just
reporting an
irritating inconsistency in the handling of tickets, biased against
well-meaning but insufficiently clued contributors (which are the sort of
people I''d be wanting to encourage, not discourage, if it were my
project).

- Matt

[1] one of the bugs I found I didn''t even have to run the code to find
it --
a desk check was enough to make it perfectly obvious.
-- 
Microsoft: We took the "perfect" out of "Wordperfect"

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> Is it the policy of the Rails project that defect reports without a
> verifiable test case are invalid and will be closed?
>
I guess that''ll be a nice policy to have considering number of open
tickets trac has. Well, that seems to be an undocumented policy, a
good thing anyways.
> I''m not sure it''s a practical proposition
> -- most people don''t "get" testing, and plenty of
Rails'' users aren''t
> sufficiently good programmers to be able to write a solid test case for
> something as complex as Rails.
>
Let them use core mailing list then. Which is a perfect medium.
> So mark them "more info required" with a note that they need to
have more
> info attached or they''ll be closed in N days/weeks/months.
>
That''d be same as closing as "incomplete".
> <advocate type="devil">
>  That cuts both ways, though -- I, as a developer, don''t have time
to debug a
>  problem in Rails.  That problem isn''t mine -- I didn''t
write the buggy code
>  that''s causing the hassle.  It''s *your* fault, dammit.
> </advocate>
Oh well. That wasn''t really needed. May be you can elaborate on *your*
part.
> Going back to my original point, though, about actively *closing* tickets
> that document *real* bugs that are trivially verifiable[1] simply because
> the patch isn''t sufficient is only reasonable in one circumstance
-- if
> defect reports without test cases are summarily closed.  I haven''t
seen that
> policy documented anywhere (and I would imagine something that it would
want
> to be in big letters on the front page), and in fact the exact opposite is
> stated on the dev front page -- "Tickets are fine".  I''m
just reporting an
> irritating inconsistency in the handling of tickets, biased against
> well-meaning but insufficiently clued contributors (which are the sort of
> people I''d be wanting to encourage, not discourage, if it were my
project).
>
Perfect in theory. But practically speaking, if there is a problem
that affects majority of people, it won''t go unnoticed. And otherwise,
people can always use this very mailing list. The reason why I''m in
favor of actively closing tickets without patches/verifying tests is
the current state of trac. 100s of open tickets. And a tiny % of them
are valid bugs. And because of that, many of the proper patches go
unnoticed for long time. They just get "lost".

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> (Good thing you put ''usually'' in there, because
I''ve got a patch in the
> system that fixes a bug in the *test suite* that''s been hanging
for over a
> week now)
It''s not on the verified patches list, so it''s unlikely to get
much attention.

http://dev.rubyonrails.org/report/12
> Is it the policy of the Rails project that defect reports without a
> verifiable test case are invalid and will be closed?
No, defect reports without test cases aren''t supposed to be
automatically closed.  A great way for people to help is to find
defect reports, then attach failing test cases, or at the very least
confirm that they experience the same behaviour.

However I believe you''re overstating the impact of closing a defect.
There are two possibilities when someone reports a bug:

a) The bug was invalid, and caused either by the user''s system, the
alignment of jupiter''s moons or some other freak occurrence
b) The bug is valid, someone else will be able to verify it

Prematurely closing defects has no adverse impact on the first case.
In the second case, the submitter can simply reopen them.   Now even
if you assume that closing the ticket has so completely enraged the
submitter that they''ve gone off to write web frameworks in ocaml,
we''re still ok.  Someone else will hit the same bug, and will either
reopen the original report (unlikely because trac''s search is shit) or
submit a new one.

The new ticket can then be fixed, or we can repeat this process again.

The reality is that there are bugs in every system, and if there''s a
defect which is so rare that only one person has ever hit it, then I''m
not going to lose any sleep over continuing to ship that bug.
> > The problem is we have tons of unverified (and bogus)
> > defect reports with no easy way to prove them.
>
> So mark them "more info required" with a note that they need to
have more
> info attached or they''ll be closed in N days/weeks/months.
That''s exactly the same as closing them, as they''re unlikely
to
continue to receive any attention.  I think lying to our submitters by
leaving an issue ''open'' when no one intends to even look at it
is
worse than closing it prematurely.
> <advocate type="devil">
>  That cuts both ways, though -- I, as a developer, don''t have time
to debug a
>  problem in Rails.  That problem isn''t mine -- I didn''t
write the buggy code
>  that''s causing the hassle.  It''s *your* fault, dammit.
> </advocate>
>
> In reality, I know all about the OSS project philosophy, I''m a
great
> believer in it myself.  If you want support, take out a support contract,
> and all that.
That''s a complete mischaracterisation of the situation we''re
in, I''m
more than willing to help people fix the problems they have.  I spend
a good portion of my time doing just that.

We have too many reports,  so many that it''s impossible to make a dent
in them.  Reporters hear nothing back about their bug report, get
disenchanted and leave.  The patch queue fills up to the point we
don''t know where to start.   If we get better turnaround time on
patches and encourage contributors, and in the process prematurely
close a few edge-case defects.   So be it.
> Going back to my original point, though, about actively *closing* tickets
> that document *real* bugs that are trivially verifiable[1] simply because
> the patch isn''t sufficient is only reasonable in one circumstance
-- if
> defect reports without test cases are summarily closed.  I haven''t
seen that
> policy documented anywhere (and I would imagine something that it would
want
> to be in big letters on the front page), and in fact the exact opposite is
> stated on the dev front page -- "Tickets are fine".  I''m
just reporting an
> irritating inconsistency in the handling of tickets, biased against
> well-meaning but insufficiently clued contributors (which are the sort of
> people I''d be wanting to encourage, not discourage, if it were my
project).
Indeed, while reports without failing test cases aren''t as likely to
get attention, they''re still valuable information, and could provide
indications of where to start contributing for people who are keen to
help out.  Please don''t close them unless you''ve tried to
reproduce
the bug, and are unable to do so.

However I don''t buy the sky is falling hypothesis of closing a ticket
prematurely,  just reopen it, explain it''s still a bug.  If the
disagreement continues, start a thread here.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Jul 14, 6:57 pm, Pratik <pratikn...@gmail.com>
wrote:
> Perfect in theory. But practically speaking, if there is a problem
> that affects majority of people, it won''t go unnoticed. And
otherwise,
> people can always use this very mailing list. The reason why I''m
in
> favor of actively closing tickets without patches/verifying tests is
> the current state of trac. 100s of open tickets. And a tiny % of them
> are valid bugs. And because of that, many of the proper patches go
> unnoticed for long time. They just get "lost".
Definitely proves the old system was flawed. I think over the best
couple month, the turnaround time for newly opened tickets has greatly
improved. We should keep doing what we are doing now in terms of new
tickets. I think the question thats concerning both of us is, what
should we do with all the old stale tickets? Rails as 1076 open
tickets at the moment. Still alot to go through by hand. (This is
probably a new thread).

I''m pretty happy with the requirements we have stated right now. We
can let those set for another month or so before we try to do
something stricter like require a patch for every ticket (which I
won''t be opposed too).


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> I''m pretty happy with the requirements we have stated right now.
We
> can let those set for another month or so before we try to do
> something stricter like require a patch for every ticket (which I
> won''t be opposed too).
Tickets describing bugs are a great asset.  It lets us know if
something''s broken, and also lets people who want to ''chip
in'' help
out.

Rather than closing bug reports, try to reproduce the bugs that are
mentioned.   If you can''t, then close the ticket telling the submitter
what you tried, and that it worked fine for you.  If you successfully
reproduce the ticket, you can probably whip up a test case, or even a
fix.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

I feel your pain here...perhaps it would be better if keywords were
used to remove things from the normal report and put them only in the
reports where they belong (e.g., undocumented == the "Undocumented
Patches", same with untested, etc.).  That way it''s not
"closed" but
the submitter is told that more information is needed.

--Jeremy

On 7/14/07, Matthew Palmer <mpalmer@hezmatt.org>
wrote:
>
> On Sat, Jul 14, 2007 at 08:30:30AM -0700, Kevin Clark wrote:
> >
> > On 7/14/07, Matthew Palmer <mpalmer@hezmatt.org> wrote:
> > > I have a particular
> > > definition of what ''closed'' means to me, and
it''s not "OK, time to write a
> > > test case".
> > >
> > > - Matt
> >
> > Does the fact that it''s being said that a closure
isn''t a death
> > sentence, just a matter of feedback change your opinion of
''closed''?
>
> No, because it''s an emotional response, not a purely intellectual
one.
>
> > Do the ''untested'', ''undocumented''
resolutions hurt less?
>
> They''re not "resolutions", they''re states.  You
set them as flags or
> something.  The very meaning of word "resolution" implies some
sort of
> finality, but a defect report that isn''t fixed isn''t
resolved.
>
> - Matt
>
> --
> Skippy was a wallaby. ... Wallabies are dumb and not very trainable...  The
> *good* thing...is that one Skippy looks very much like all the rest,
> hence..."one-shot Skippy" and "plug-compatible Skippy".
I don''t think they
> ever had to go as far as "belt-fed Skippy"      -- Robert
Sneddon, ASR
>
> >
>

-- 
http://www.jeremymcanally.com/

My free Ruby e-book:
http://www.humblelittlerubybook.com/book/

My blogs:
http://www.mrneighborly.com/
http://www.rubyinpractice.com/

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Jul 15, 12:24 am, "Jeremy McAnally"
<jeremymcana...@gmail.com>
wrote:
> I feel your pain here...perhaps it would be better if keywords were
> used to remove things from the normal report and put them only in the
> reports where they belong (e.g., undocumented == the "Undocumented
> Patches", same with untested, etc.).  That way it''s not
"closed" but
> the submitter is told that more information is needed.
Thats the process that left us with 1000 open tickets. Lots of people
don''t respond or fix their ticket. This is just lets us with a bunch
of inactive tickets in the queue. Close the ticket now, it says closed
if the person abandons it later and if they are truly interested and
getting it through, can can easily reopen it.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

It seems the main issues are:

1. We have to many open, stale tickets.
2. If we close tickets we will hurt peoples feelings/people think that
a closed ticket is a permanent state and that it should not be
reopened.

IMHO the simple solution is that when one closes a ticket, the ticket
comment should simply say something like "This ticket is being closed
due to xxxxx. Feel free to repoen this tick once xxxx has been added,
or to discuss the merits of this action. Thanks for taking the time to
help your community!"

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Jul 14, 4:38 pm, Matthew Palmer <mpal...@hezmatt.org> wrote:
> <advocate type="devil">
>  ...
> </advocate>
>> advocate.is_a? Devil
=> false
>> advocate.for? Devil
=> true


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Jul 15, 6:49 am, "Steven A Bristol" <stevenbris...@gmail.com>
wrote:
> It seems the main issues are:
>
> 1. We have to many open, stale tickets.
> 2. If we close tickets we will hurt peoples feelings/people think that
> a closed ticket is a permanent state and that it should not be
> reopened.
>
> IMHO the simple solution is that when one closes a ticket, the ticket
> comment should simply say something like "This ticket is being closed
> due to xxxxx. Feel free to repoen this tick once xxxx has been added,
> or to discuss the merits of this action. Thanks for taking the time to
> help your community!"
+1 on Steve''s idea here.  Having this little extra text should smooth
things a little for those of us with less thick skin. :)

Do these steps apply for documentation as well?  I''m not sure how to
write a test for a documentation change. :)  Is that possible?

Mike B.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Sun, Jul 15, 2007 at 08:46:17PM -0700, Mike Barsalou
wrote:
> On Jul 15, 6:49 am, "Steven A Bristol"
<stevenbris...@gmail.com>
> wrote:
> > It seems the main issues are:
> >
> > 1. We have to many open, stale tickets.
> > 2. If we close tickets we will hurt peoples feelings/people think that
> > a closed ticket is a permanent state and that it should not be
> > reopened.
> >
> > IMHO the simple solution is that when one closes a ticket, the ticket
> > comment should simply say something like "This ticket is being
closed
> > due to xxxxx. Feel free to repoen this tick once xxxx has been added,
> > or to discuss the merits of this action. Thanks for taking the time to
> > help your community!"
> 
> +1 on Steve''s idea here.  Having this little extra text should
smooth
> things a little for those of us with less thick skin. :)
I have reasonably thick skin.  I''ll also note that the tickets involved
*did* have a message about what needed to be fixed and done to get the patch
accepted.

The thing that got me going was that a report of a material defect in Rails
was handled more harshly *because* it had a patch.  Say what you like about
how closing a ticket isn''t the end of the world, it''s pretty
simple to
understand that if my tickets get shuffled off the "Bugs which
haven''t been
[...] fixed" page only when I provide a patch, I''m less likely to
submit
patches and I''ll just stick to reporting the bugs.

What I''m agitating for is one of two things:

* Set a clear and obvious policy that defect reports without a complete test
  case will be closed.  I''ve noticed lifofifo went and closed a bunch
of
  tickets with "Please attach tests/patch", but reopened them again a
couple
  of hours later.  Presumably the policy isn''t to reject tickets
without
  test cases, then?

*OR*

* Change the policy that defect reports with incomplete patches get closed. 
  Comment on them, set ''patchstatus=incomplete'' or a
''patch-incomplete'' tag
  or whatever, but don''t *close* them.  They''re valid defect
reports and
  should be placed with the rest of the valid defect reports, wherever that
  may choose to be.  (Handling of invalid defect reports is the same whether
  there''s a patch attached or not).

I hope I''ve made myself clear this time around.  Defect reports,
inasmuch as
they document a defect, should not be treated differently based on whether
there''s a patch.  The patch handling process is a separate issue.
> Do these steps apply for documentation as well?  I''m not sure how
to
> write a test for a documentation change. :)  Is that possible?
Same thing applies to test case fixes -- there''s no test suite for the
test
suite.  Presumably with those they''d just go through the three
reviewers
process.

- Matt

-- 
Ruby''s the only language I''ve ever used that feels like it was
designed by a
programmer, and not by a hardware engineer (Java, C, C++), an academic
theorist (Lisp, Haskell, OCaml), or an editor of PC World (Python).
		-- William Morgan

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> Do these steps apply for documentation as well?  I''m not sure how
to
> write a test for a documentation change. :)  Is that possible?
>
Oh yeah! Spellcheck ;)

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> +1 on Steve''s idea here.  Having this little extra text should
smooth
> things a little for those of us with less thick skin. :)
Definitely,  someone should start a wiki page with a few
''canned''
paragraphs that can evolve over time.
> Do these steps apply for documentation as well?  I''m not sure how
to
> write a test for a documentation change. :)  Is that possible?
The review steps do.  But obviously until something invents
assert_makes_sense_and_explains_the_concept_well we''ll have to skip
the automated testing.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> I hope I''ve made myself clear this time around.  Defect reports,
inasmuch as
> they document a defect, should not be treated differently based on whether
> there''s a patch.  The patch handling process is a separate issue.
I see what you''re saying here, and you''re clearer this time
around, or
(more likely) I''m less caffeine deprived.

The reality is that patches and defects are already handled in a very
different manner.  We want quick turnaround time on patches to keep
submitters happy, however defects are generally less time sensitive.

I''ve seen two potential solutions here,  either open another ticket
when uploading a patch and reference the defect report.  Or simply
reopen the ticket with a comment saying "This is still a bug".

Ideally when rejecting a patch that''s attached to a ''bug
ticket'' we
can simply get rid of the [PATCH] prefix to have it excluded from the
reports.
> > Do these steps apply for documentation as well?  I''m not sure
how to
> > write a test for a documentation change. :)  Is that possible?
>
> Same thing applies to test case fixes -- there''s no test suite for
the test
> suite.  Presumably with those they''d just go through the three
reviewers
> process.
Same deal, get the reviews done, and obviously make sure the tests
still pass :).

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Mon, Jul 16, 2007 at 08:59:56PM +1200, Michael Koziarski
wrote:
> > I hope I''ve made myself clear this time around.  Defect
reports, inasmuch as
> > they document a defect, should not be treated differently based on
whether
> > there''s a patch.  The patch handling process is a separate
issue.
> 
> I see what you''re saying here, and you''re clearer this
time around, or
> (more likely) I''m less caffeine deprived.
Unless *everyone''s* caffeine deprived, I don''t think
I''ve been clear.
> I''ve seen two potential solutions here,  either open another
ticket
> when uploading a patch and reference the defect report.  Or simply
> reopen the ticket with a comment saying "This is still a bug".
OK, I''ll reopen tickets with that comment if they get closed for dodgy
patches.
> Ideally when rejecting a patch that''s attached to a ''bug
ticket'' we
> can simply get rid of the [PATCH] prefix to have it excluded from the
> reports.
Well, if the patch isn''t valid, the ticket doesn''t really have
a patch, so
that''s perfectly reasonable.

- Matt

-- 
A few minutes ago I attempted to give a flying fsck, but the best I could do
was to watch it skitter across the floor.
		-- Anthony de Boer, ASR

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Jul 14, 2:44 am, Pratik <pratikn...@gmail.com>
wrote:
> Sounds cool.
>
> You can probably mention #rails-contrib as well for finding potential
reviewer.
>
> And how about having a rule to close the ticket as invalid/wontfix if
> it''s been there for over 2-3 months without any updates ? And may
be
> get a core member to reopen it if one feels the patch has a chance of
> making it to core ever. I was just going through some of the old
> tickets, and I feel that very small % of them will ever make it to
> core. Probably closing the remaining tickets would be good to keep
> things clean on trac as some of them are even 2 years old.
Hi, is it feasible to add a ''tiimeout/stale'' option for the
closing,
rather than invalid/wontfix.  At least then you''d be able to see more
accurately why something was closed ?  It also opens up the
possibility of having a bot of some sort just sweep daily and time
things out (while still retaining some traceability of the fact it was
auto-closed rather than manually closed).

A.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---