Re: Submitted Patches

[Igor Vaynberg <ig...@gmail.com>]

On Tue, Dec 23, 2008 at 7:11 PM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
> Is there anything special I need to do after submitting a patch for a bug so
> that it gets included?  Currently I have three patches submittted to random
> bugs / requests, two of which have been there for quite some time with no
> comment and noone applying them.
>
> WICKET-1513 - bug that I think the patch is safe and should be committed
> WICKET-1916 - bug that I think the patch is safe and should be committed

this patch messes with url processing which is very fragile in wicket.
how extensively have you tested this because there are probably ten
bugs open right now for similar issues. we fix one and another one
pops up, so we are very careful when it comes to messing with stuff
like this. are you sure that patch didnt break any of the other
hundred usecases that have to do with ajax and ie6 on tomcat vs ie7 on
jetty? i hope juergen tested it extensively before applying.

> WICKET-1883 - I could see why this one may not be committed - it's huge, and
> Igor's already said it would probably wait until 1.5 (which is a shame,
> because I'm sure by then the patch won't work any more).

it wasnt applied because like you said you havent tested it
extensively. so the burden is on us and we would much rather work on
bugs then things marked as "wish". another reason it wasnt applied was
because we now have plans to rewrite that part of wicket entirely in
1.5.

> WICKET-1567

this wasnt applied yet because it has to do with 1.3 and compatibility
issues. are you sure there are no deployed wicket applications out
there that will break after this patch is applied because they depend
on existing behavior?

> Anyway - since I worked to fix those bugs, I don't want to see them fall
> through the cracks and would like to see them make it into future releases.

sure, but you also have to look at it from out point of view. you
write the patch which takes x time. we have to maintain that code
after you are done, which takes x*1000 time. so just because there is
a patch doesnt mean it will be applied. if you really want to help you
should concentrate on bugs for trunk. if you want to work on a "wish"
type issue or a 1.3  bug that changes runtime behavior you should let
it be known on the dev@ so we can make sure it is something we will
want to maintian.

thanks for your contributions.

-igor

>
> Thanks!
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
>

Re: Submitted Patches

[Igor Vaynberg <ig...@gmail.com>]

the only requirement is that you attach a patch in a unified format so
that it is easy for us to apply. we run the unit tests ourselves and
also test the functionality. if something doesnt work or the patch
doesnt apply cleanly we simply reject it or rework it. so, to answer
your question: no, there isnt anything special.

-igor

On Fri, Jan 2, 2009 at 9:19 AM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
> Thanks for the responses Igor.  Please refer back to the original question:
>
> "Is there anything special I need to do after submitting a patch for a bug
> so that it gets included?"
> I'm not pissed off - so don't read into what's not there (that's the problem
> with email - inadequate communication of emotion).  I simply wanted to see
> if there was a step I was missing.  I'm just trying to be a valuable asset
> to the community.
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
>
> On Fri, Jan 2, 2009 at 10:41 AM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> On Fri, Jan 2, 2009 at 8:04 AM, Jeremy Thomerson
>> <je...@wickettraining.com> wrote:
>> >> this patch messes with url processing which is very fragile in wicket.
>> >> how extensively have you tested this because there are probably ten
>> >> bugs open right now for similar issues. we fix one and another one
>> >> pops up, so we are very careful when it comes to messing with stuff
>> >> like this. are you sure that patch didnt break any of the other
>> >> hundred usecases that have to do with ajax and ie6 on tomcat vs ie7 on
>> >> jetty? i hope juergen tested it extensively before applying.
>> >
>> >
>> > I ran all tests with "mvn clean test" and tested what I could in a
>> > quickstart.  What else would you suggest?
>>
>> this patch has to do with ajax functionality. i would suggest running
>> the attached quickstart on all javascript engines that we support:
>> ie6.0, ie6.5, ie7, safari (whatever two major versions there are out
>> there), firefox 2, firefox 3, opera, blah blah blah blah. than test
>> running it on tomcat 5, tomcat 6, jetty, etc. see why we are careful
>> with stuff like this? there are a lot of variables that contribute to
>> issues like this and our automated tests do not cover all of them. so
>> its not unusual for an issue like this to sit open for a long time,
>> until one of the core devs has an entire day to kill on it.
>>
>> >> > WICKET-1567
>> >>
>> >> this wasnt applied yet because it has to do with 1.3 and compatibility
>> >> issues. are you sure there are no deployed wicket applications out
>> >> there that will break after this patch is applied because they depend
>> >> on existing behavior?
>> >
>> >
>> > It's a *bug* in 1.3.  Since it was fixed in trunk, I assumed that the
>> core
>> > devs had already considered it "safe" for future upgrades.
>>
>> we do not care about breaking backwards compat in trunk nearly as much
>> as in 1.3.x releases. the rules there are different.
>>
>> > No - we're never
>> > sure if there are "any" apps that will break, but this seems very
>> innocuous.
>> >
>> > Besides - what "existing behavior" could they be depending on - it
>> currently
>> > generates an invalid link that doesn't work?
>>
>> we have come across a lot of issues while working on stable branches
>> where we fix something trivial like this - which hardly affects anyone
>> - and break a behavior that someone else has coded against. the
>> severity of this bug is very low and there are easy workarounds, so we
>> have to weight that against a chance of breaking something already
>> deployed.
>>
>> >> > Anyway - since I worked to fix those bugs, I don't want to see them
>> fall
>> >> > through the cracks and would like to see them make it into future
>> >> releases.
>> >>
>> >> sure, but you also have to look at it from out point of view. you
>> >> write the patch which takes x time. we have to maintain that code
>> >> after you are done, which takes x*1000 time. so just because there is
>> >> a patch doesnt mean it will be applied. if you really want to help you
>> >> should concentrate on bugs for trunk. if you want to work on a "wish"
>> >> type issue or a 1.3  bug that changes runtime behavior you should let
>> >> it be known on the dev@ so we can make sure it is something we will
>> >> want to maintian.
>> >
>> >
>> > I guess this is something I don't understand.  Since the core devs are
>> > focused on getting another high-quality release out (1.4), it would seem
>> to
>> > me that you would rather welcome true bug fixes for the 1.3 branch, which
>> is
>> > probably (my guess only) the most-deployed version out there.
>>
>> only if you understand the restrictions that apply to making code
>> changes in 1.3 branch.
>>
>> > I understand that not all patches will be committed, but what else can I
>> do
>> > to help besides go through issues filed as bugs, fix them, run all tests
>> and
>> > submit the patch?
>>
>> not get pissed off when your patch is not applied right away or for a while
>> :)
>>
>> > What else do you do personally when you are fixing bugs?
>>
>> when it comes to 1.3 branch i am a lot more conscientious of what code
>> i am changing and how it affects deployed apps. there were more then a
>> few times when i had to rollback the changes i committed when someone
>> pointed out that it breaks compatibility in some way. when dealing
>> with ajax bugs i do a lot more testing then just the automated tests
>> (which is why i usually assign ajax related stuff to matej :) ).
>>
>> -igor
>>
>> >
>> >
>> >>
>> >>
>> >> thanks for your contributions.
>> >>
>> >> -igor
>> >>
>> >> >
>> >> > Thanks!
>> >> >
>> >> > --
>> >> > Jeremy Thomerson
>> >> > http://www.wickettraining.com
>> >> >
>> >>
>> >
>> >
>> >
>> > --
>> > Jeremy Thomerson
>> > http://www.wickettraining.com
>> >
>>
>

Re: Submitted Patches

[Jeremy Thomerson <je...@wickettraining.com>]

Thanks for the responses Igor.  Please refer back to the original question:

"Is there anything special I need to do after submitting a patch for a bug
so that it gets included?"
I'm not pissed off - so don't read into what's not there (that's the problem
with email - inadequate communication of emotion).  I simply wanted to see
if there was a step I was missing.  I'm just trying to be a valuable asset
to the community.

-- 
Jeremy Thomerson
http://www.wickettraining.com

On Fri, Jan 2, 2009 at 10:41 AM, Igor Vaynberg <ig...@gmail.com>wrote:

> On Fri, Jan 2, 2009 at 8:04 AM, Jeremy Thomerson
> <je...@wickettraining.com> wrote:
> >> this patch messes with url processing which is very fragile in wicket.
> >> how extensively have you tested this because there are probably ten
> >> bugs open right now for similar issues. we fix one and another one
> >> pops up, so we are very careful when it comes to messing with stuff
> >> like this. are you sure that patch didnt break any of the other
> >> hundred usecases that have to do with ajax and ie6 on tomcat vs ie7 on
> >> jetty? i hope juergen tested it extensively before applying.
> >
> >
> > I ran all tests with "mvn clean test" and tested what I could in a
> > quickstart.  What else would you suggest?
>
> this patch has to do with ajax functionality. i would suggest running
> the attached quickstart on all javascript engines that we support:
> ie6.0, ie6.5, ie7, safari (whatever two major versions there are out
> there), firefox 2, firefox 3, opera, blah blah blah blah. than test
> running it on tomcat 5, tomcat 6, jetty, etc. see why we are careful
> with stuff like this? there are a lot of variables that contribute to
> issues like this and our automated tests do not cover all of them. so
> its not unusual for an issue like this to sit open for a long time,
> until one of the core devs has an entire day to kill on it.
>
> >> > WICKET-1567
> >>
> >> this wasnt applied yet because it has to do with 1.3 and compatibility
> >> issues. are you sure there are no deployed wicket applications out
> >> there that will break after this patch is applied because they depend
> >> on existing behavior?
> >
> >
> > It's a *bug* in 1.3.  Since it was fixed in trunk, I assumed that the
> core
> > devs had already considered it "safe" for future upgrades.
>
> we do not care about breaking backwards compat in trunk nearly as much
> as in 1.3.x releases. the rules there are different.
>
> > No - we're never
> > sure if there are "any" apps that will break, but this seems very
> innocuous.
> >
> > Besides - what "existing behavior" could they be depending on - it
> currently
> > generates an invalid link that doesn't work?
>
> we have come across a lot of issues while working on stable branches
> where we fix something trivial like this - which hardly affects anyone
> - and break a behavior that someone else has coded against. the
> severity of this bug is very low and there are easy workarounds, so we
> have to weight that against a chance of breaking something already
> deployed.
>
> >> > Anyway - since I worked to fix those bugs, I don't want to see them
> fall
> >> > through the cracks and would like to see them make it into future
> >> releases.
> >>
> >> sure, but you also have to look at it from out point of view. you
> >> write the patch which takes x time. we have to maintain that code
> >> after you are done, which takes x*1000 time. so just because there is
> >> a patch doesnt mean it will be applied. if you really want to help you
> >> should concentrate on bugs for trunk. if you want to work on a "wish"
> >> type issue or a 1.3  bug that changes runtime behavior you should let
> >> it be known on the dev@ so we can make sure it is something we will
> >> want to maintian.
> >
> >
> > I guess this is something I don't understand.  Since the core devs are
> > focused on getting another high-quality release out (1.4), it would seem
> to
> > me that you would rather welcome true bug fixes for the 1.3 branch, which
> is
> > probably (my guess only) the most-deployed version out there.
>
> only if you understand the restrictions that apply to making code
> changes in 1.3 branch.
>
> > I understand that not all patches will be committed, but what else can I
> do
> > to help besides go through issues filed as bugs, fix them, run all tests
> and
> > submit the patch?
>
> not get pissed off when your patch is not applied right away or for a while
> :)
>
> > What else do you do personally when you are fixing bugs?
>
> when it comes to 1.3 branch i am a lot more conscientious of what code
> i am changing and how it affects deployed apps. there were more then a
> few times when i had to rollback the changes i committed when someone
> pointed out that it breaks compatibility in some way. when dealing
> with ajax bugs i do a lot more testing then just the automated tests
> (which is why i usually assign ajax related stuff to matej :) ).
>
> -igor
>
> >
> >
> >>
> >>
> >> thanks for your contributions.
> >>
> >> -igor
> >>
> >> >
> >> > Thanks!
> >> >
> >> > --
> >> > Jeremy Thomerson
> >> > http://www.wickettraining.com
> >> >
> >>
> >
> >
> >
> > --
> > Jeremy Thomerson
> > http://www.wickettraining.com
> >
>

Re: Submitted Patches

[Igor Vaynberg <ig...@gmail.com>]

On Fri, Jan 2, 2009 at 8:04 AM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
>> this patch messes with url processing which is very fragile in wicket.
>> how extensively have you tested this because there are probably ten
>> bugs open right now for similar issues. we fix one and another one
>> pops up, so we are very careful when it comes to messing with stuff
>> like this. are you sure that patch didnt break any of the other
>> hundred usecases that have to do with ajax and ie6 on tomcat vs ie7 on
>> jetty? i hope juergen tested it extensively before applying.
>
>
> I ran all tests with "mvn clean test" and tested what I could in a
> quickstart.  What else would you suggest?

this patch has to do with ajax functionality. i would suggest running
the attached quickstart on all javascript engines that we support:
ie6.0, ie6.5, ie7, safari (whatever two major versions there are out
there), firefox 2, firefox 3, opera, blah blah blah blah. than test
running it on tomcat 5, tomcat 6, jetty, etc. see why we are careful
with stuff like this? there are a lot of variables that contribute to
issues like this and our automated tests do not cover all of them. so
its not unusual for an issue like this to sit open for a long time,
until one of the core devs has an entire day to kill on it.

>> > WICKET-1567
>>
>> this wasnt applied yet because it has to do with 1.3 and compatibility
>> issues. are you sure there are no deployed wicket applications out
>> there that will break after this patch is applied because they depend
>> on existing behavior?
>
>
> It's a *bug* in 1.3.  Since it was fixed in trunk, I assumed that the core
> devs had already considered it "safe" for future upgrades.

we do not care about breaking backwards compat in trunk nearly as much
as in 1.3.x releases. the rules there are different.

> No - we're never
> sure if there are "any" apps that will break, but this seems very innocuous.
>
> Besides - what "existing behavior" could they be depending on - it currently
> generates an invalid link that doesn't work?

we have come across a lot of issues while working on stable branches
where we fix something trivial like this - which hardly affects anyone
- and break a behavior that someone else has coded against. the
severity of this bug is very low and there are easy workarounds, so we
have to weight that against a chance of breaking something already
deployed.

>> > Anyway - since I worked to fix those bugs, I don't want to see them fall
>> > through the cracks and would like to see them make it into future
>> releases.
>>
>> sure, but you also have to look at it from out point of view. you
>> write the patch which takes x time. we have to maintain that code
>> after you are done, which takes x*1000 time. so just because there is
>> a patch doesnt mean it will be applied. if you really want to help you
>> should concentrate on bugs for trunk. if you want to work on a "wish"
>> type issue or a 1.3  bug that changes runtime behavior you should let
>> it be known on the dev@ so we can make sure it is something we will
>> want to maintian.
>
>
> I guess this is something I don't understand.  Since the core devs are
> focused on getting another high-quality release out (1.4), it would seem to
> me that you would rather welcome true bug fixes for the 1.3 branch, which is
> probably (my guess only) the most-deployed version out there.

only if you understand the restrictions that apply to making code
changes in 1.3 branch.

> I understand that not all patches will be committed, but what else can I do
> to help besides go through issues filed as bugs, fix them, run all tests and
> submit the patch?

not get pissed off when your patch is not applied right away or for a while :)

> What else do you do personally when you are fixing bugs?

when it comes to 1.3 branch i am a lot more conscientious of what code
i am changing and how it affects deployed apps. there were more then a
few times when i had to rollback the changes i committed when someone
pointed out that it breaks compatibility in some way. when dealing
with ajax bugs i do a lot more testing then just the automated tests
(which is why i usually assign ajax related stuff to matej :) ).

-igor

>
>
>>
>>
>> thanks for your contributions.
>>
>> -igor
>>
>> >
>> > Thanks!
>> >
>> > --
>> > Jeremy Thomerson
>> > http://www.wickettraining.com
>> >
>>
>
>
>
> --
> Jeremy Thomerson
> http://www.wickettraining.com
>

Re: Submitted Patches

[Jeremy Thomerson <je...@wickettraining.com>]

Notes inline.

On Fri, Jan 2, 2009 at 8:21 AM, Igor Vaynberg <ig...@gmail.com>wrote:

> On Tue, Dec 23, 2008 at 7:11 PM, Jeremy Thomerson
> <je...@wickettraining.com> wrote:
> > Is there anything special I need to do after submitting a patch for a bug
> so
> > that it gets included?  Currently I have three patches submittted to
> random
> > bugs / requests, two of which have been there for quite some time with no
> > comment and noone applying them.
> >
> > WICKET-1513 - bug that I think the patch is safe and should be committed
> > WICKET-1916 - bug that I think the patch is safe and should be committed
>
> this patch messes with url processing which is very fragile in wicket.
> how extensively have you tested this because there are probably ten
> bugs open right now for similar issues. we fix one and another one
> pops up, so we are very careful when it comes to messing with stuff
> like this. are you sure that patch didnt break any of the other
> hundred usecases that have to do with ajax and ie6 on tomcat vs ie7 on
> jetty? i hope juergen tested it extensively before applying.


I ran all tests with "mvn clean test" and tested what I could in a
quickstart.  What else would you suggest?


>
> > WICKET-1883 - I could see why this one may not be committed - it's huge,
> and
> > Igor's already said it would probably wait until 1.5 (which is a shame,
> > because I'm sure by then the patch won't work any more).
>
> it wasnt applied because like you said you havent tested it
> extensively. so the burden is on us and we would much rather work on
> bugs then things marked as "wish". another reason it wasnt applied was
> because we now have plans to rewrite that part of wicket entirely in
> 1.5.


Yes, agree with this one completely.  This was just something I worked on
while at ApacheCon US Hack-a-Thon.  No worries.


>
> > WICKET-1567
>
> this wasnt applied yet because it has to do with 1.3 and compatibility
> issues. are you sure there are no deployed wicket applications out
> there that will break after this patch is applied because they depend
> on existing behavior?


It's a *bug* in 1.3.  Since it was fixed in trunk, I assumed that the core
devs had already considered it "safe" for future upgrades.  No - we're never
sure if there are "any" apps that will break, but this seems very innocuous.

Besides - what "existing behavior" could they be depending on - it currently
generates an invalid link that doesn't work?


>
>
> > Anyway - since I worked to fix those bugs, I don't want to see them fall
> > through the cracks and would like to see them make it into future
> releases.
>
> sure, but you also have to look at it from out point of view. you
> write the patch which takes x time. we have to maintain that code
> after you are done, which takes x*1000 time. so just because there is
> a patch doesnt mean it will be applied. if you really want to help you
> should concentrate on bugs for trunk. if you want to work on a "wish"
> type issue or a 1.3  bug that changes runtime behavior you should let
> it be known on the dev@ so we can make sure it is something we will
> want to maintian.


I guess this is something I don't understand.  Since the core devs are
focused on getting another high-quality release out (1.4), it would seem to
me that you would rather welcome true bug fixes for the 1.3 branch, which is
probably (my guess only) the most-deployed version out there.

I understand that not all patches will be committed, but what else can I do
to help besides go through issues filed as bugs, fix them, run all tests and
submit the patch?  What else do you do personally when you are fixing bugs?


>
>
> thanks for your contributions.
>
> -igor
>
> >
> > Thanks!
> >
> > --
> > Jeremy Thomerson
> > http://www.wickettraining.com
> >
>



-- 
Jeremy Thomerson
http://www.wickettraining.com