We practice atomic commits that change only one thing, and in one way. We separate Fix, Refactor, Change, Add, Move and Remove commits from each other. Our commit summaries begin with those keywords so we can tell what "type" the change is. Each commit must pass CI 100% on it's own.
There are specific characteristics we've noticed. For example, for us, a "pure Refactor" commit either touches code or tests, but not both at the same time. If we touch too much we try to split it, or we call it a Change and it gets even more/different scrutiny.
Reviewing by commit allows me to give my full focus to each without having to keep all state in my head at once. I can step through and understand the series of changes. It's like telling a story, once I've stepped through each commit I am better able to understand the whole picture. If I only look at the summary some of the finer points are missed.
Also we've found we can extract commits more easily and get those merged early while we work on our main branches. If we see a Refactor, we can just do it, extract it and then keep our main PR focused.
More important to me, the "logically split" commits are less useful when hunting down a bug months or years later. They explain the final intent, but not the path to get there, when the path to get there reveals when the bug was actually introduced, and following the thought process using those original commits reveals what the developer was intending instead of just what the end result was.
For example, was the bug introduced in the refactoring/cleanup? This happens a lot, and an earlier state of the code reveals what it should be doing. Was it introduced during initial development? If so, is it just an edge case they didn't think about (because it should have been impossible and a recent change elsewhere made it possible), or is it a remnant of some earlier version of the feature? ..etc
(That remnant-of-an-earlier-version one has actually happened to me and straight removing some code rather than trying to fix it was the correct way to handle it)
We're working on FLOSS projects where contributors (internal and external) are asked to split commits logically and that's how they end up in the final history. Most merge requests consist of just a couple of small commits, although sometimes there's a bigger and longer-lived branch where it is easier to just look at the diff as a whole (separate commits still massively improve bisectability though and I can't think of any real downsides of having them retained).
This is sort of a meaningless question without an understanding of how often people commit. Some people keep a single commit for a single PR (and just constantly update that one commit), others make tiny little commits for every individual change.
The only real question is: what is the granularity you should review code changes at? The number of git commits that that maps to doesn't really matter.
IMO: the ideal is to try to keep pull requests as small as possible while still having each PR be a coherent, justifiable change on its own. I don't know if it's realistic to treat that as a hard rule, but I think it's the right thing to aim for.
The answer for me at last is no. Because the history in a PR should not matter but only the final commit. That's why we choose to squash commits in my current role.
Wip commits we’re often and frequent for CI purposes but as with PRs you don’t want to waste too much review time on WIP unless your asking for some preliminary feedback on a complex change.
Ideal commit length was unchanged to any other commit length you might be expected to do on any other team with good commit hygiene.