> On Jul 6, 2017, at 4:22 PM, Justin Mclean <jus...@classsoftware.com> wrote:
> 
> Hi,
> 
>> - Do we care about code being commented out? I think not.
> 
> If we know why it was commented out sure. There’s always the concern is this 
> code possibly needed or not?

That is the concern, but it may be that the developer was making a speculative 
change. If I were cleaning then I would do one of two actions.

(a) Review the history of that change and see if the associated comments 
provide a reason.
(b) Ask the developer who made the change.

> 
>> - Does a semicolon matter?
> 
> It marked as a minor issue but it can cause issues when JS code is optimised 
> and/or minified. It’s easy and safe to fix.

While very likely safe I think that a question needs to be asked.

> 
>> - Is “:Object” really a critical issue?
> 
> Critical no (and it's not marked as such) and in some cases it’s quite 
> useful. Like all rules there are exceptions. Using a class with named 
> properties gives you type safety and performance benefits.

Well Object is always THE base class. It may not be known what the actual base 
class should be and it is certainly not flexible to always create a base class 
just to have a one to satisfy the rule. I think again it is a question that I 
think was already answered by Alex. IIRC he did not think these warranted a 
different approach.


Justin - If you have clear performance results then please document them in the 
wiki.

> 
>> - Is refactoring “complexity” really a huge problem
> 
> If we want easy to test/easy to maintain code then it can be yes.

This is a question of long term maintenance. My experience in an Agile 
environment is that these changes must be planned otherwise they become break 
fixes. Refactoring must be very carefully done. The author may have this 
complexity there for an unmentioned reason.

> 
>> - This one: Replace == with === : has been discussed and I think your making 
>> the change suggested broke things.
> 
> It didn’t break things in any major way, the tests passed, examples worked 
> and the application I’m working on worked. I believe there was one instance 
> that caused an issue for Harbs.

OK. Well I recall that and I also recall several people doubting that the 
change was needed or necessary. Perhaps both sides could be put on a wiki page.

> 
> There are some other useful rules in there I’ll document them.

Thanks. Do you know how to deactivate useless rules?

> 
>> Probably the best approach would be to collect these into a wiki page and 
>> then determine what conventions developers consider important. Once that is 
>> done change the sonar configuration and show true technical debt. I think 
>> that this is much less risky to the project and much less distraction to the 
>> team.
> 
> Sure I’ll go with that approach.

Wonderful.

Regards,
Dave

> 
> Thanks,
> Justin

Attachment: signature.asc
Description: Message signed with OpenPGP

Reply via email to