Move PHP default threshold to 28 #97
Conversation
When analyzing the WordPress project at the current default mass threshold of 10, it was observed that Duplications found only by Platform had masses of 27 or less, while Duplications found by both Platform and Classic had masses of 28 or greater. Note: there were no Duplications found only by Classic. Therefore, a threshold of 28 should ensure an analysis on Platform emits the same Duplication issues as on Classic.
pbrisbin
force-pushed
the
pb-php-default-threshold
branch
from
7ddb13c to
944ce64
Compare
|
LGTM |
|
@pbrisbin sounds good for threshold change. Since you're changing that, can you also change the points per from |
|
@ABaldwinHunter I'd rather make smaller individual changes. I haven't looked at tuning points yet, just working on the threshold. |
|
@pbrisbin okay suit yourself. I'm surprised to see specs pass for php |
Move PHP default threshold to 28
|
"When analyzing the WordPress project at the current default mass threshold of 10, it was observed that Duplications found only by Platform had masses of 27 or less, while Duplications found by both Platform and Classic had masses of 28 or greater. Note: there were no Duplications found only by Classic." Confused by this explanation: is there anything surprising about this behavior? Seems like that's how threshold is supposed to work? |
|
Did you expect surprising? It's just the justification for choosing 28 vs On Mon, Feb 8, 2016, 19:18 Noah Davis [email protected] wrote:
|
|
It seems like you're saying "I picked 28 because it's what we had on
|
|
The masses are different between platform and classic, so it's not quite I picked the mass that, in platform, caused the same instances of On Mon, Feb 8, 2016, 21:09 Noah Davis [email protected] wrote:
|
|
Yeah sorry, not nit-picking English just trying to understand your thought Honestly I'm still confused, but if it's clearer in person don't want to To me it seems like you're describing behavior that is the way it is by I'm also confused that you state that the masses differ between classic and Sorry if I'm uber confused/confusing things. On Mon, Feb 8, 2016 at 9:21 PM pat brisbin [email protected] wrote:
|
|
I'll try to clarify tomorrow when not on my phone. If we need to chat in On Mon, Feb 8, 2016, 21:55 Noah Davis [email protected] wrote:
|
|
So the problem here is I was confused :) Basically, I went through this process: View duplications in a file under Platform analysis
(Note: these numbers are made up) Compare the same file under Classic analysis
This told me a few things:
Then I moved to the console. There I listed out all the masses in Platform and Classic for this project (WordPress/WordPress). I found that instances found in both case were all (platform) mass 28 or more and all instances found only in platform had (platform) mass 27 or less. This is how I arrived at 28. Unfortunately, I never looked at the classic code base. I assumed that, since the masses are different, the threshold must be different. And since I'd just determined one analytically, I didn't think I needed to. Turns out, classic does use 28! @noahd1 knew that, and assumed I knew that, and my attempts at explanation were that much more confusing. So how can that be? If the masses are different, why does the same threshold work? Well, originally I noticed the difference in mass between platform and classic in the UI. I never actually compared them beyond that (even in my console analysis, I was only looking at platform masses and which instances appeared in one or both sets). Turns out, classic is actually showing an "effective" mass in that UI; one which takes number of occurrences and identical-vs-similar into account. In actuality, platform and classic produce the same "raw" mass, and so they can and should use the same threshold. My analysis in the console did result in the right number (28) and the fact that it does match classic (now that we know that, and know why) further bolsters its appropriateness. And I apologize for not fully understanding what was going on and therefore being unable to explain things correctly. |
When analyzing the WordPress project at the current default mass threshold of
10, it was observed that Duplications found only by Platform had masses of 27
or less, while Duplications found by both Platform and Classic had masses of 28
or greater. Note: there were no Duplications found only by Classic.
Therefore, a threshold of 28 should ensure an analysis on Platform emits the
same Duplication issues as on Classic.
/cc @codeclimate/review @noahd1
I think this could be shipped without a Changelog. It should cause a small
uptick in GPA. For example, WordPress went from 0.85 to 0.88.