You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by Yasser Zamani <ya...@live.com> on 2017/02/04 18:19:47 UTC
Re: How to select how to solve issue?
I have fixed WW-4694 where I found that I need some recommendation
before creating it's PR, to fix it as best as possible.
In general, would you like to issue being solved with fewer changes as
much as possible? Or no, as best as possible but with some more changes?
In more specific:
1. In a PR, Do you recommend to add an unit test which tests that
specific issue or similar issues? Or no, make current unit tests passing
is enough and recommended?
2. When I fixed WW-4694, I discovered that our AnnotationUtils has not
any tangible improvement for about 4 years and also, with my changes, a
method named isAnnotatedBy will be almost useless or duplicate. But
Spring has a similar class with same name and purpose which has been
wrote more robust and much newer than our one. I'm sure I can merge them
carefully with respect to not breaking the rest of the current codes and
also, merge only until my changes get clear, beauty and known future
bugs free, not merge everything. But is it recommended by you? Or no,
you prefer fewer changes but with higher possibility of future bugs and
maybe ugly code as changes made day by day?
Thanks in advance for your reading!
On 2/3/2017 2:23 AM, Yasser Zamani wrote:
> Thank you Aleksandr, I made my first one
> at https://github.com/apache/struts/pull/116 😉
>
>
> Thank you Lukasz for your explanation.
>
>
> Thanks all.
>
>
> Regards,
>
> Yasser.
>
>
>
> ------------------------------------------------------------------------
> *From:* Aleksandr Mashchenko <am...@apache.org>
> *Sent:* Wednesday, February 1, 2017 8:40 PM
> *To:* dev@struts.apache.org
> *Subject:* Re: How to select which issue to work on?
>
> Hi,
>
> > does 'unassigned' mean no one already work on it?
> Usually, yes.
>
> > is it better to solve old reported one or newer ones?
> Pick the one you can solve. Just make sure that particular issue is
> still relevant.
>
> Looking forward to see PR-s from you.
>
> ---
> Regards,
> Aleksandr
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
Re: How to select how to solve issue?
Posted by Rene Gielen <rg...@apache.org>.
Am 13.02.17 um 12:10 schrieb Lukasz Lenart:
> 2017-02-04 19:19 GMT+01:00 Yasser Zamani <ya...@live.com>:
>> 2. When I fixed WW-4694, I discovered that our AnnotationUtils has not
>> any tangible improvement for about 4 years and also, with my changes, a
>> method named isAnnotatedBy will be almost useless or duplicate. But
>> Spring has a similar class with same name and purpose which has been
>> wrote more robust and much newer than our one. I'm sure I can merge them
>> carefully with respect to not breaking the rest of the current codes and
>> also, merge only until my changes get clear, beauty and known future
>> bugs free, not merge everything. But is it recommended by you? Or no,
>> you prefer fewer changes but with higher possibility of future bugs and
>> maybe ugly code as changes made day by day?
>
> It also depends :) If it is possible to resolve issue without coping,
> that's good, but if there is already a fix at hand and the solution
> isn't too specific (which can harm users at some point) I don't mind
> borrowing code ;-)
>
>
To get more into detail here: Copying from other OS licensed code is ok
as long as all requirements are met.
First off, it must be an OS license that is compatible with ASL V2.0.
See https://www.apache.org/legal/resolved#category-a for a complete list.
Second, we have to comply with specific license requirements of the
original work. In case of Spring both is quite easy, since they are
using ASL 2.0 themselves.
The important section in ASL 2.0 is cited below. As long theses
requirements are met, derived code is ok to be included legally.
- Ren�
4. Redistribution. You may reproduce and distribute copies of the Work
or Derivative Works thereof in any medium, with or without
modifications, and in Source or Object form, provided that You meet the
following conditions:
You must give any other recipients of the Work or Derivative Works a
copy of this License; and
You must cause any modified files to carry prominent notices stating
that You changed the files; and
You must retain, in the Source form of any Derivative Works that You
distribute, all copyright, patent, trademark, and attribution notices
from the Source form of the Work, excluding those notices that do not
pertain to any part of the Derivative Works; and
If the Work includes a "NOTICE" text file as part of its distribution,
then any Derivative Works that You distribute must include a readable
copy of the attribution notices contained within such NOTICE file,
excluding those notices that do not pertain to any part of the
Derivative Works, in at least one of the following places: within a
NOTICE text file distributed as part of the Derivative Works; within the
Source form or documentation, if provided along with the Derivative
Works; or, within a display generated by the Derivative Works, if and
wherever such third-party notices normally appear. The contents of the
NOTICE file are for informational purposes only and do not modify the
License. You may add Your own attribution notices within Derivative
Works that You distribute, alongside or as an addendum to the NOTICE
text from the Work, provided that such additional attribution notices
cannot be construed as modifying the License.
You may add Your own copyright statement to Your modifications and may
provide additional or different license terms and conditions for use,
reproduction, or distribution of Your modifications, or for any such
Derivative Works as a whole, provided Your use, reproduction, and
distribution of the Work otherwise complies with the conditions stated
in this License.
--
Ren� Gielen
http://twitter.com/rgielen
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: How to select how to solve issue?
Posted by Lukasz Lenart <lu...@apache.org>.
2017-02-04 19:19 GMT+01:00 Yasser Zamani <ya...@live.com>:
> I have fixed WW-4694 where I found that I need some recommendation
> before creating it's PR, to fix it as best as possible.
>
> In general, would you like to issue being solved with fewer changes as
> much as possible? Or no, as best as possible but with some more changes?
Depends :) If it is possible, smaller steps is my prefered way
(resolve one then another issue) but sometimes there is no other
option than do it as a huge refactoring. Targeting one issue on PR is
the way to go, even if the PR gets large.
> In more specific:
>
> 1. In a PR, Do you recommend to add an unit test which tests that
> specific issue or similar issues? Or no, make current unit tests passing
> is enough and recommended?
It's enough but adding additional tests which cover the new issue is
the best approach (not always feasible). It would be nice to have a
unit test which fails when issue is not fixed (it can be an existing
unit test which was modified to properly test code without issue)
> 2. When I fixed WW-4694, I discovered that our AnnotationUtils has not
> any tangible improvement for about 4 years and also, with my changes, a
> method named isAnnotatedBy will be almost useless or duplicate. But
> Spring has a similar class with same name and purpose which has been
> wrote more robust and much newer than our one. I'm sure I can merge them
> carefully with respect to not breaking the rest of the current codes and
> also, merge only until my changes get clear, beauty and known future
> bugs free, not merge everything. But is it recommended by you? Or no,
> you prefer fewer changes but with higher possibility of future bugs and
> maybe ugly code as changes made day by day?
It also depends :) If it is possible to resolve issue without coping,
that's good, but if there is already a fix at hand and the solution
isn't too specific (which can harm users at some point) I don't mind
borrowing code ;-)
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: how to solve issue? fewer changes or...
Posted by Lukasz Lenart <lu...@apache.org>.
2017-02-07 9:50 GMT+01:00 Yasser Zamani <ya...@live.com>:
> I also discovered sometimes PRs will be dependent e.g. PR2 will fix
> ISSUE2 if and only if PR1 has been accepted and pulled before. I think I
> should fix ISSUE2 in PR1 too and links them in JIRA, all of these before
> PR1's acceptation, to avoid make multiple dependent PRs with shared
> changes, right?
If fixing an issue solves two or many JIRA tickets that's ok to link
them to the same PR, if a PR fixes more than one independent issue
this isn't a good idea.
Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
how to solve issue? fewer changes or...
Posted by Yasser Zamani <ya...@live.com>.
Thank you so much for useful comments.
I also wait for Lukasz, Aleksandr and others for any comments and then
will update my PR to satisfy that it's a helpful PR as much as possible.
I also discovered sometimes PRs will be dependent e.g. PR2 will fix
ISSUE2 if and only if PR1 has been accepted and pulled before. I think I
should fix ISSUE2 in PR1 too and links them in JIRA, all of these before
PR1's acceptation, to avoid make multiple dependent PRs with shared
changes, right?
On 2/6/2017 2:22 PM, Christoph Nenning wrote:
> Hi,
>
> in general I prefer to have better and bigger PRs :)
>
>
>> 1. In a PR, Do you recommend to add an unit test which tests that
>> specific issue or similar issues? Or no, make current unit tests passing
>
>> is enough and recommended?
>
>
> I love having more tests ;)
> Tests for specific issues are alright.
>
>
>
>> 2. When I fixed WW-4694, I discovered that our AnnotationUtils has not
>> any tangible improvement for about 4 years and also, with my changes, a
>> method named isAnnotatedBy will be almost useless or duplicate. But
>> Spring has a similar class with same name and purpose which has been
>> wrote more robust and much newer than our one. I'm sure I can merge them
>
>> carefully with respect to not breaking the rest of the current codes and
>
>> also, merge only until my changes get clear, beauty and known future
>> bugs free, not merge everything. But is it recommended by you? Or no,
>> you prefer fewer changes but with higher possibility of future bugs and
>> maybe ugly code as changes made day by day?
>
>
> Copying code from other projects my cause legal issues. You should avoid
> doing that. But it would be great to refactor and improve AnnotationUtils.
> If old methods can be removed it is great, too.
>
>
>
> Regards,
> Christoph
>
>
>
>> From: Yasser Zamani <ya...@live.com>
>> To: Struts Developers List <de...@struts.apache.org>,
>> Date: 04.02.2017 19:19
>> Subject: Re: How to select how to solve issue?
>>
>> I have fixed WW-4694 where I found that I need some recommendation
>> before creating it's PR, to fix it as best as possible.
>>
>> In general, would you like to issue being solved with fewer changes as
>> much as possible? Or no, as best as possible but with some more changes?
>>
>> In more specific:
>>
>> 1. In a PR, Do you recommend to add an unit test which tests that
>> specific issue or similar issues? Or no, make current unit tests passing
>
>> is enough and recommended?
>>
>> 2. When I fixed WW-4694, I discovered that our AnnotationUtils has not
>> any tangible improvement for about 4 years and also, with my changes, a
>> method named isAnnotatedBy will be almost useless or duplicate. But
>> Spring has a similar class with same name and purpose which has been
>> wrote more robust and much newer than our one. I'm sure I can merge them
>
>> carefully with respect to not breaking the rest of the current codes and
>
>> also, merge only until my changes get clear, beauty and known future
>> bugs free, not merge everything. But is it recommended by you? Or no,
>> you prefer fewer changes but with higher possibility of future bugs and
>> maybe ugly code as changes made day by day?
>>
>> Thanks in advance for your reading!
>>
>> On 2/3/2017 2:23 AM, Yasser Zamani wrote:
>>> Thank you Aleksandr, I made my first one
>>> at https://github.com/apache/struts/pull/116 😉
>>>
>>>
>>> Thank you Lukasz for your explanation.
>>>
>>>
>>> Thanks all.
>>>
>>>
>>> Regards,
>>>
>>> Yasser.
>>>
>>>
>>>
>>>
> ------------------------------------------------------------------------
>>> *From:* Aleksandr Mashchenko <am...@apache.org>
>>> *Sent:* Wednesday, February 1, 2017 8:40 PM
>>> *To:* dev@struts.apache.org
>>> *Subject:* Re: How to select which issue to work on?
>>>
>>> Hi,
>>>
>>> > does 'unassigned' mean no one already work on it?
>>> Usually, yes.
>>>
>>> > is it better to solve old reported one or newer ones?
>>> Pick the one you can solve. Just make sure that particular issue is
>>> still relevant.
>>>
>>> Looking forward to see PR-s from you.
>>>
>>> ---
>>> Regards,
>>> Aleksandr
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>>> For additional commands, e-mail: dev-help@struts.apache.org
>>>
>>
>> ---
>> This email has been checked for viruses by Avast antivirus software.
>> https://www.avast.com/antivirus
>>
>
>
> This Email was scanned by Sophos Anti Virus
>
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: How to select how to solve issue?
Posted by Christoph Nenning <Ch...@lex-com.net>.
Hi,
in general I prefer to have better and bigger PRs :)
> 1. In a PR, Do you recommend to add an unit test which tests that
> specific issue or similar issues? Or no, make current unit tests passing
> is enough and recommended?
I love having more tests ;)
Tests for specific issues are alright.
> 2. When I fixed WW-4694, I discovered that our AnnotationUtils has not
> any tangible improvement for about 4 years and also, with my changes, a
> method named isAnnotatedBy will be almost useless or duplicate. But
> Spring has a similar class with same name and purpose which has been
> wrote more robust and much newer than our one. I'm sure I can merge them
> carefully with respect to not breaking the rest of the current codes and
> also, merge only until my changes get clear, beauty and known future
> bugs free, not merge everything. But is it recommended by you? Or no,
> you prefer fewer changes but with higher possibility of future bugs and
> maybe ugly code as changes made day by day?
Copying code from other projects my cause legal issues. You should avoid
doing that. But it would be great to refactor and improve AnnotationUtils.
If old methods can be removed it is great, too.
Regards,
Christoph
> From: Yasser Zamani <ya...@live.com>
> To: Struts Developers List <de...@struts.apache.org>,
> Date: 04.02.2017 19:19
> Subject: Re: How to select how to solve issue?
>
> I have fixed WW-4694 where I found that I need some recommendation
> before creating it's PR, to fix it as best as possible.
>
> In general, would you like to issue being solved with fewer changes as
> much as possible? Or no, as best as possible but with some more changes?
>
> In more specific:
>
> 1. In a PR, Do you recommend to add an unit test which tests that
> specific issue or similar issues? Or no, make current unit tests passing
> is enough and recommended?
>
> 2. When I fixed WW-4694, I discovered that our AnnotationUtils has not
> any tangible improvement for about 4 years and also, with my changes, a
> method named isAnnotatedBy will be almost useless or duplicate. But
> Spring has a similar class with same name and purpose which has been
> wrote more robust and much newer than our one. I'm sure I can merge them
> carefully with respect to not breaking the rest of the current codes and
> also, merge only until my changes get clear, beauty and known future
> bugs free, not merge everything. But is it recommended by you? Or no,
> you prefer fewer changes but with higher possibility of future bugs and
> maybe ugly code as changes made day by day?
>
> Thanks in advance for your reading!
>
> On 2/3/2017 2:23 AM, Yasser Zamani wrote:
> > Thank you Aleksandr, I made my first one
> > at https://github.com/apache/struts/pull/116 😉
> >
> >
> > Thank you Lukasz for your explanation.
> >
> >
> > Thanks all.
> >
> >
> > Regards,
> >
> > Yasser.
> >
> >
> >
> >
------------------------------------------------------------------------
> > *From:* Aleksandr Mashchenko <am...@apache.org>
> > *Sent:* Wednesday, February 1, 2017 8:40 PM
> > *To:* dev@struts.apache.org
> > *Subject:* Re: How to select which issue to work on?
> >
> > Hi,
> >
> > > does 'unassigned' mean no one already work on it?
> > Usually, yes.
> >
> > > is it better to solve old reported one or newer ones?
> > Pick the one you can solve. Just make sure that particular issue is
> > still relevant.
> >
> > Looking forward to see PR-s from you.
> >
> > ---
> > Regards,
> > Aleksandr
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> > For additional commands, e-mail: dev-help@struts.apache.org
> >
>
> ---
> This email has been checked for viruses by Avast antivirus software.
> https://www.avast.com/antivirus
>
This Email was scanned by Sophos Anti Virus