Allow marking duplications as "false-positives"/"won't fix"

I’ve recent run into a situation where a code-block was duplicated by design. And now, our integration pipeline is failing because the new commits trigger a quality-gate fail due to this duplication.

We write code which is used to interact with network devices and one of the vendors recently spit their running OS on those devices. They now have a separate life-cycle. Part of our code was duplicated to represent this split, so we can evolve both code-bases independently.

It would be nice to be able to flag that duplication as “won’t fix”.

We could remove the duplication, but it was an internal design-decision to duplicate it. But this decision is now blocking the integration pipeline so we will remove the duplication again. Even though it’s not quite what we want. But at least this way the pipeline will pass.

It would be nice to be able to flag duplications as “won’t fix” for some of these corner cases.

Hi,

Could you elaborate on the QG condition that’s failing? Is it using one of the duplications metrics, or is it a condition related to issues?

In general, there’s no way to mark anything metric-related as FP/WF. The only way to do that is to challenge the counting algorithm & persuade us to change it. :slight_smile:

 
Ann

This is something we would like implemented as well…

The only type of example that keeps popping up for us is around JSON objects in an array, for example:

var arr = [
	{"foo": 1, "bar": 2},
	{"foo": 3, "bar": 4},
	{"foo": 5, "bar": 6}
]

The views with our development team are pretty split about this, half saying it is duplications, half saying it isn’t. The people that have a problem with duplications being flagged on this is when they refactor their code it becomes more complex and harder to read. The benefit being though, you know all properties are consistent across all objects.

var arr = [];

function pushObject(foo, bar) {
	arr.push({"foo": foo, "bar": bar);
}

pushObject(1,2);
pushObject(3,4);
pushObject(5,6);

But like the OP says, we have no choice but to do it via the 2nd methods because otherwise the quality gate has been failed and there is no way to get around it apart from exclude the whole file for coverage analysis, which is not what we want to do.

@ganncamp @Fabrice_Bellingard Is there any consideration to my post above?

Thanks,
Pete

Hi Pete,

I don’t see a lot to be done here other than to recommend you exclude the file from duplication calculation, rather than coverage :slight_smile:, and to recommend that you reconsider your quality gate conditions. If you’ve made a conscious decision to introduce a specific duplication then you shouldn’t have to undo what you’ve decided is the best code practice just to pass the QG. Either your QG threshold needs adjusting, or you need to give yourself a one-time pass to release with a red QG.

If you’re going to tell me release is an automated process that can’t move forward with a red QG, well… that’s tough, but assuming your QG is focused on New Code values (it is, isn’t it?) you should be able to work around that.

Regarding the actual low-level duplications calculation, basically you’re asking that we update the duplication detection algorithm for… JavaScript? only? to ignore a certain type of object syntax, while admitting that even your own team is split on the topic. I’m sure you can see why that’s kinda a hard sell.

 
Ann

Hi Ann,

Thanks for coming back to me. We discussed the idea of excluding the file, but there might be code that is duplicated in there that then wouldn’t get picked up ‘legitimately’. I guess for JavaScript we could move the array to a file, exclude it and then just request it, but for other languages that wouldn’t work so well.

We don’t have an automated process for now, we review the quality gates at the end of each sprint, so that isn’t a problem and we are happy to release with a one-time pass, however, if the duplication occurs on a common bit of code, we find ourselves having to raise exemptions and releasing a failed quality gate on most releases.

The point I was putting forward was for object declarations inside an array across all programming languages, not just JavaScript. But it has most commonly happened for us in JavaScript, as this is the case, I think I will put forward the idea of moving the array to an independent JSON file and then just excluding the whole file.

Thanks,
Pete

@ganncamp there is another use case here that I think needs to be considered. With the introduction of the properly quality gates on PR / short-lived branches in 7.7, we now have developers running into problems with small changes breaking the quality gate because of duplicated code. For example, I have one who’s changed 34 lines of code and two lines are being picked up as duplicates (a new property in two different classes). Since a positive quality decoration is required to approve the PR, without the ability to mark the duplication as won’t fix we have no way of completing the PR without changing the quality gate.

1 Like