[openstack-dev] [Fuel][Fuel-QA][Fuel-TechDebt] Code Quality: Do Not Hardcode - Fix Things Instead
Igor Kalnitsky
ikalnitsky at mirantis.com
Wed Nov 11 21:27:57 UTC 2015
Folks,
I have one thing to add: if workaround is unavoidable, please DO
comment it. Usually workaround aren't obvious, and it would be
incredibly helpful to comment all of them; and do not hesitate to
write extensive comments. The clearer you write - the less time your
colleagues will spend next time they touch such code.
Thanks,
Igor
On Wed, Nov 11, 2015 at 2:58 AM, Vladimir Kuklin <vkuklin at mirantis.com> wrote:
> Matthew
>
> Thanks for your feedback. Could you please elaborate more on the statistics
> of such tech-debt eliminations? My perception is that such bugs do not ever
> get fixed actually jeopardizing our efforts on bugfixing and actually making
> our statistics manupilative.
>
> So far my suggestion is the following - if you can, please do not introduce
> workarounds. If you have - introduce a TODO/FIXME comment for it in the code
> and create a tech-debt bug. If you see something of that kind that is
> already there and does not have such a comment - add this TODO/FIXME and
> create a tech-debt bug.
>
> So this is a best effort initiative, but I would encourage core reviewers to
> be stricter with such workarounds and hacks - please, do not get them pass
> through your hands unless there is a really good reason to merge this code
> with these hacks right now.
>
> On Wed, Nov 11, 2015 at 1:43 PM, Matthew Mosesohn <mmosesohn at mirantis.com>
> wrote:
>>
>> Vladimir,
>>
>> Bugfixes and minor refactoring often belong in separate commits. Combining
>> "extending foo to enable bar in XYZ" with "ensuring logs from service abc
>> are sent via syslog" often makes little sense to code reviewers. In this
>> case it is a feature enhancement + a bugfix.
>>
>> Looking at it from one perspective, if the bugfix is made poorly without a
>> feature commit, then it looks like the scenario you described. However, it
>> has the benefit that it can be cleanly backported. If we simply reverse the
>> order of the commits (untangling the workaround), we get the same result,
>> but get flamed.
>>
>> Sometimes both approaches are necessary. I agree that not growing tech
>> debt is important, but perceptions really depend on trends over 3+ weeks.
>> It's possible that such tech debt bugs are created and solved within 2-3
>> days of the workaround. I know that's the exception, but I think we should
>> be most concerned about what happens when we carry tech debt across entire
>> Fuel releases.
>>
>> On Nov 11, 2015 10:28 AM, "Aleksandr Didenko" <adidenko at mirantis.com>
>> wrote:
>>>
>>> +1 from me
>>>
>>> On Tue, Nov 10, 2015 at 6:38 PM, Stanislaw Bogatkin
>>> <sbogatkin at mirantis.com> wrote:
>>>>
>>>> I think that it is excellent thought.
>>>> +1
>>>>
>>>> On Tue, Nov 10, 2015 at 6:52 PM, Vladimir Kuklin <vkuklin at mirantis.com>
>>>> wrote:
>>>>>
>>>>> Folks
>>>>>
>>>>> I wanted to raise awareness about one of the things I captured while
>>>>> doing reviews recently - we are sacrificing quality to bugfixing and feature
>>>>> development velocity, essentially moving from one heap to another - from
>>>>> bugs/features to 'tech-debt' bugs.
>>>>>
>>>>> I understand that we all have deadlines and need to meet them. But,
>>>>> folks, let's create the following policy:
>>>>>
>>>>> 1) do not introduce hacks/workarounds/kludges if it is possible.
>>>>> 2) while fixing things if you have a hack/workaround/kludge that you
>>>>> need to work with - think of removing it instead of enhancing and extending
>>>>> it. If it is possible - fix it. Do not let our technical debt grow.
>>>>> 3) if there is no way to avoid kludge addition/enhancing, if there is
>>>>> no way to remove it - please, add a 'TODO/FIXME' line above it, so that we
>>>>> can collect them in the future and fix them gradually.
>>>>>
>>>>> I suggest to add this requirement into code-review policy.
>>>>>
>>>>> What do you think about this?
>>>>>
>>>>> --
>>>>> Yours Faithfully,
>>>>> Vladimir Kuklin,
>>>>> Fuel Library Tech Lead,
>>>>> Mirantis, Inc.
>>>>> +7 (495) 640-49-04
>>>>> +7 (926) 702-39-68
>>>>> Skype kuklinvv
>>>>> 35bk3, Vorontsovskaya Str.
>>>>> Moscow, Russia,
>>>>> www.mirantis.com
>>>>> www.mirantis.ru
>>>>> vkuklin at mirantis.com
>>>>>
>>>>>
>>>>> __________________________________________________________________________
>>>>> OpenStack Development Mailing List (not for usage questions)
>>>>> Unsubscribe:
>>>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>>
>>>>
>>>>
>>>>
>>>> __________________________________________________________________________
>>>> OpenStack Development Mailing List (not for usage questions)
>>>> Unsubscribe:
>>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>
>>>
>>>
>>>
>>> __________________________________________________________________________
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe:
>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>
>> __________________________________________________________________________
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
>
>
> --
> Yours Faithfully,
> Vladimir Kuklin,
> Fuel Library Tech Lead,
> Mirantis, Inc.
> +7 (495) 640-49-04
> +7 (926) 702-39-68
> Skype kuklinvv
> 35bk3, Vorontsovskaya Str.
> Moscow, Russia,
> www.mirantis.com
> www.mirantis.ru
> vkuklin at mirantis.com
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
More information about the OpenStack-dev
mailing list