This makes it impossible to review SVG diffs as text, which is critically important for Breeze icons. I think this should be reverted.

@ndavis @carlschwan

@bcooksley should we keep this?

From a server performance perspective, yes.

From a "making it possible for developers to review changes to uncompressed text-based SVG icons" perspective, no.

That's the conflict here.

In general, we do want diffs for SVGs. The code of the SVG is important to get right. The main reason this was needed in this repo recently was because the size of the following commit was too big to push. If it was spit up in 2-4 parts, it probably would have worked, but it's also a problem that there was no way to know without getting lucky or trial and error.

You do need to keep this yes, otherwise any large diff will be unable to be merged or otherwise pushed.

The only way to avoid it is to ensure that you do not have any files that have long lines in them (the number of lines shouldn't be an issue - it is the length of them that is the problem)

It is unfortunate that in the case of Breeze the SVG text actually makes sense. Usually it is auto-generated and does not make sense.

Keeping this permanently in the breeze-icons repo is not acceptable, sorry. It makes it impossible for developers to actually review merge requests. We need to come up with another solution.

Could we add it temporarily in merge requests that are unmerge-able without it, right before merge, and then immediately revert it?

Ideally the problem causing it to be required would be fixed though.

Nope it has to live on the master branch, so temporarily adding and removing it would have to be done separately on master. Cannot be done via MR.

The problem causing it is somewhere in the license checking code I believe, and given it has been untouched for years with the exception of the SPDX stuff I suspect it is the SPDX code at fault here.

Ok, we can do it separately on master then, until the root cause issue is solved.

mentioned in commit c5770db6

mentioned in commit 6783af5f