search

sinanpl / OaxacaBlinder

R implementation of Oaxaca-Blinder gap decomposition
MIT License
1 stars 1 forks source link

Set/enforce an explicit coding style for the project #37

Closed davidskalinder closed 4 months ago

davidskalinder commented 4 months ago

I think this would help with #36, and (perhaps?) with readability of the code. I know with all the PRs I've been submitting and merging, I've tried to stick to the style I can see in the existing code, but I'm not always sure which way to go and I'm sure I've made plenty of mistakes even when I am sure.

So I think it would help to choose an explicit style, ideally one that can be automatically enforced. I assume styler::tidyverse_style() is the most sensible choice? (Though I think it's not as deterministic as I'd like -- I'd prefer something like Black, but I don't think anything so strict exists for R?)

Mind you, I think it will be tough to apply a styler with so many outstanding PRs, since a full restyling will create a jillion diffs to keep up with (unless I'm missing a clever way of handling this?). But if we get to a point where master is caught up with all (or nearly all) of the PRs, I think it would be good to pick a style and run a styler for it over the codebase.

After that, I think there are ways to set up styler to automatically style incoming code? With, umm, pre-commit hooks, or GitHub actions, or other tricks like that? Something like that would probably be best, but @sinanpl I think even if you just decide on a style that I could apply with a function before committing then I'd feel a lot more confident that I wasn't screwing up a bunch of merges just by choosing the wrong indentation style...

davidskalinder commented 4 months ago

Hmph, I just noticed that styler still doesn't have a solution for long lines, which IMO decreases its value for this somewhat. Still better than nothing I guess?...

sinanpl commented 4 months ago

Closing. Github action is set up which will style & commit based on the styler package at any push