Fix codecov warning due to too low fetch depth

[ste93cry]

Since a while I noticed that when I force push the commits of a PR we get the following warning in the GitHub Actions log and no Codecov status check is shown anymore:

Issue detecting commit SHA. Please run actions/checkout with fetch-depth > 1 or set to 0

According to codecov/codecov-action#190 we should definitely set fetch-depth to something greater than 1. As a side note, I also switched the indentation of the YAML files from 4 spaces to 2 as it looks like it's the common standard

[Jean85]

I'm 👍 wit this change but, as I said elsewhere, IMO the solution would also be NOT to push-force: we already do merge-squash, so altering the history of a PR is just a problem, since it makes it harder for incremental reviewing, and also for stuff like this Codecov hiccup.

[stayallive]

I have the same comment as @Jean85 it's really annoying to review something and after changes all GitHub links are dead and I can't see what someone changed after a force push so really that should be the preferred way to handle this.

[ste93cry]

we already do merge-squash, so altering the history of a PR is just a problem, since it makes it harder for incremental reviewing

Merging the target branch into the PR creates the same issue as not squash-merging once the PR is ready: a rebase is usually preferred because the merge create merge commits that make the git history a lot harder to read. When I had to review your PRs where you merged the branch, the history contained the commits of the target branch in addition to yours which drove me crazy

[Jean85]

Merging the target branch into the PR creates the same issue as not squash-merging once the PR is ready: a rebase is usually preferred because the merge create merge commits that make the git history a lot harder to read. When I had to review your PRs where you merged the branch, the history contained the commits of the target branch in addition to yours which drove me crazy

This depends a lot on which tool you use for your reviews. Here on GitHub forcing maims all the links an makes it impossible to review it incrementally.

[ste93cry]

makes it impossible to review it incrementally

I don't understand why you say this, rebasing does not mean that the commits are lost... Anyway, it's just a matter of preference, some people prefers to rebase and others to merge. I always saw people going with the first way, but this doesn't mean that the second is wrong 😉

[Jean85]

Commits are not lost but are completely rewritten, so I do not longer know where I was last time that I reviewed; when you rebase you rewrite the entire history of the branch, so I have to review the whole diff from the start.

[EDIT] To give you a perfect example that I just stumbled into: you just force-pushed a commit twice on getsentry/sentry-symfony#430, and I got two notifications with a commit link:

• getsentry/sentry-symfony@d09af90
• getsentry/sentry-symfony@c353ce6

Both have:

• the same commit message
• but different content!
• are marked as: This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository
• have lost reference to the original PR

So now I have to guess what you've done (force pushed to fix the CI maybe?) and I don't know what you did in regard to where I left the PR the last time that I saw and reviewed it, because I no longer have a clear starting point for a partial diff.

[ste93cry]

That's still not true, if you mark the files as "viewed" and nothing changed GitHub does not mark them as having new changes. This worked quite well for me in the past

got two notifications

Yes, that's a downside of this approach: notification links may point to old commits, but as I said once you start marking the files as viewed, if you open the PR it should work fine and you should see only files that really changed content.

[Jean85]

Ok, I'll try with the "Viewed" feature