struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From martin.coo...@tumbleweed.com
Subject Re: [PATCH] FormTag - call reset after instantiation
Date Mon, 17 Sep 2001 16:35:23 GMT
Erik,

I apologise if I offended by not making the exact changes you posted.
Actually, there were two reasons for not applying the patch as-is.

The first is that the patch, as copied from the archive, could not be
applied directly. The patch utility complained about it, and wouldn't do it.
I thought it must be something to do with formatting, and fiddled with it
for a while, but then gave up and made the changes manually.

The second is directly related to the first, in that if the patch had worked
as is, I would have left it alone. In general, I agree with you that
refactoring and cleaning up are good things to do. However, I guess I'm not
convinced that creating two new methods, each containing a single line of
code, is that helpful. I felt it was more clear to use those lines of code
directly, particularly as they are frequently seen across the Struts code
base, and there was nothing different happening here.

By the way, a good thing to do with patches is to attach them, as a file, to
the related bug report. That way, the content of the file (especially white
space and line wrapping) are preserved, and the patch should be able to be
applied directly.

Again, I apologise if I offended - that was certainly not my intent. And
thanks for the patch!

--
Martin Cooper


----- Original Message -----
From: "Erik Hatcher" <jakarta-struts@ehatchersolutions.com>
To: <struts-dev@jakarta.apache.org>
Sent: Monday, September 17, 2001 3:52 AM
Subject: Re: [PATCH] FormTag - call reset after instantiation


> Martin,
>
> Thanks for applying my patch.  I haven't tested it yet, but I had one
issue
> to bring up regarding the patch you applied.   You did not apply my patch
as
> I had implemented it exactly.   One of the things I did was to refactor
> slightly so that getting the action mappings was pulled into a separate
> method, as well as the findMapping method since both of these were done in
> other parts of the code as well.   While what you applied appears fine
> functionality-wise, I do think that refactoring and cleaning up some code
at
> the same time is a worthy goal.
>
> Is there a reason my patch wasn't applied as-is?
>
> Thanks,
>     Erik
>
>
> ----- Original Message -----
> From: <martin.cooper@tumbleweed.com>
> To: <struts-dev@jakarta.apache.org>
> Sent: Sunday, September 16, 2001 10:02 PM
> Subject: Re: [PATCH] FormTag - call reset after instantiation
>
>
> > I just checked in this change. Please let me know if you see any
problems.
> >
> > --
> > Martin Cooper
> >
> >
> > ----- Original Message -----
> > From: "Erik Hatcher" <jakarta-struts@ehatchersolutions.com>
> > To: <struts-dev@jakarta.apache.org>
> > Sent: Saturday, September 08, 2001 8:44 PM
> > Subject: Re: [PATCH] FormTag - call reset after instantiation
> >
> >
> > > Followup: I just checked Bugzilla at:
> > >     http://nagoya.apache.org/bugzilla/show_bug.cgi?id=2108
> > >
> > > It says its been applied and the issue closed, but I don't see it CVS.
> > It
> > > seems that the patch slipped through the cracks somehow.
> > >
> > > Could a committer look into this?
> > >
> > > Thanks again,
> > >     Erik
> > >
> > >
> > > ----- Original Message -----
> > > From: "Erik Hatcher" <jakarta-struts@ehatchersolutions.com>
> > > To: <struts-dev@jakarta.apache.org>
> > > Sent: Saturday, September 08, 2001 8:38 PM
> > > Subject: [PATCH] FormTag - call reset after instantiation
> > >
> > >
> > > > I'd like to lobby for this previously submitted patch to be applied:
> > > >
> http://www.mail-archive.com/struts-dev@jakarta.apache.org/msg02556.html
> > > >
> > > > Thanks,
> > > >     Erik
> > > >
> > > >
> > > >
> > >
> >
> >
> >
>



Mime
View raw message