[Table] Replace font icon to svg icon in sort label

[kybarg]

This makes icons font dependency optional
Fixes #6196

[oliviertassinari]

I will definitly add a visual regression tests on the table component. Removing the font dependency sounds good to me.

[mbrookes]

Looks good to me. 👍

[kybarg]

So merge it?)

[mbrookes]

@kybarg That's not how it works.

[oliviertassinari]

@kybarg Thanks!

That's not how it works.

I don't even know how it's work. Here is my perspective:
We need at least one core contributor to accept a PR so we merge it. That works fine for simple changes, for more challenging one or controversial, a second one accepting the PR is better.

[mbrookes]

We need at least one core contributor to accept a PR so we merge it. That works fine for simple changes, for more challenging one or controversial, a second one accepting the PR is better.

Exactly. The first reviewer accepts, second person gives a secondary review and merges. Ideally that would be the case for every PR (I've seen one-line changes with unintended consequences), but with a small team, that isn't practical, so it's a judgement call by the first reviewer whether to merge a change they approved, or leave it for a second review.

Sometimes the first reviewer will merge a PR after a period of time if no-one else has, and if there haven't been any objections raised.

Not sure this has ever been written down, but it's how the process has evolved naturally with the core team.

[oliviertassinari]

@mbrookes I think that we should be writing it down, and you have :). The audience is primarily for maintainers.