style: run package through styler

[dshemetov]

No description provided.

[brookslogan]

From a skim, looks fine. But what's the process for pending PRs? Git rebase --exec, something like this?

[brookslogan]

Oh, not sure if rebase styling would even be needed if we are merging PRs rather than rebasing PRs. [Maybe just a style commit per PR would do to prevent conflicts? But might need to think about whether rebase styling would make reviewing or future git blaming easier.]

[dshemetov]

That's a nice blog post, good find! That will certainly remove the difficulty of manually sifting through git conflicts, but having to rebase each branch is still work. If we do go down that path, I'm thinking we can have each PR owner run this locally

# Make sure you're on PR branch and rebase on dev
git checkout <branch>
git rebase origin/dev

# Rebase on ds/style
git rebase \
  --strategy-option=theirs \
  --exec 'Rscript -e "styler::style_pkg()"' \
  --exec 'git add --all' \
  --exec 'git commit --amend --no-edit' \
  origin/ds/style

# Using the unimproved version of the command above because the 
# styler command doesn't have a way to easily take output from git show

# Force push to remote
git push origin --force

This way we don't force push to an author's branch and have them run into issues with their local version.

If we don't go down the rebase path and have each PR owner make their own big styling commit, then we need everyone to add those to the ignore-revs file. On the plus side, this sounds less dangerous than rebase and force pushing. On the down side, the commit log will duplicate big formatting commits.

[brookslogan]

Non-rebase approach is also going to make reviewing a hassle if it's not done at the very very end. Might ask PR authors if we can apply the rebase approach & do that then. Locally at least we should make sure to back up the old version until everything looks good.

[brookslogan]

As I'm trying to clean up some old PRs, I'm revisiting these handy commands. Post-merge of the styler, it looks like git rebase origin/dev will complain about conflicts due to the merged styling. I might have been able to just skip that for one of the PRs, but for another, it looks like maybe it should first be rebased on 58ed6b4, the commit preceding the initial styling commit. If we use this approach as a template for other mass-restyling operations, we should probably tag that pre-initial-styling commit for convenience.