BuildHelpers icon indicating copy to clipboard operation
BuildHelpers copied to clipboard

Inconsistent code style

Open lipkau opened this issue 7 years ago • 11 comments

The functions in this repo have many different styles of code.

This is not a big problem, but a contributor using an editor that "fixes" the style (eg: VSCode), will flag a lot more changes than required.

Is it in the interest of this module to harmonize the style to avoid this? And if so, with what rules? Shall this be enforced with tests?

lipkau avatar Sep 25 '18 12:09 lipkau

Agreed, consistency would be good. Will get back to you but might be a bit (other priorities on this and other projects, summit proposal pestering, wrapping up work before parental leave, etc.).

RamblingCookieMonster avatar Sep 25 '18 12:09 RamblingCookieMonster

If we could get agreement on a primary style, PRs would be easier to submit?

pauby avatar Jan 01 '19 15:01 pauby

Not really. Primary advantages are:

  • help devs to format code by storing .vscode\settings.json in the repo
  • enable enforcing of common styling
  • easier to review changes to code, as styling is kept consistent
    • if you use vscode now and use the code formatting setting, you will re-format a lot of code without making any real changes to it

lipkau avatar Jan 01 '19 15:01 lipkau

Not really

  • enable enforcing of common styling

... Which makes submitting PRs easier (saves the submitter having to guess which style to pick) 👍

.vscode\settings.json in the repo

Or use .editorconfig so we're not tying people to an IDE. Or use both (as they are unlikely to change much).

pauby avatar Jan 01 '19 16:01 pauby

Or use .editorconfig so we're not tying people to an IDE. Or use both (as they are unlikely to change much).

This helps with other stuff... EOL char, file encoding, indention, etc... but code formatting requires more than that. to be more precise: This setting for the powershell extension in vscode https://github.com/AtlassianPS/ConfluencePS/blob/b176b2382c2551af56eec460f71140d79020ceb6/.vscode/settings.json#L12

lipkau avatar Jan 01 '19 16:01 lipkau

This helps with other stuff... EOL char, file encoding, indention, etc...

It does. It's limited. But using it stops you mandating people use VS Code (as believe it or not, not everybody does) and raising the bar for people becoming involved. For those people it's the only thing you can give them. That's why I suggested using both:

Or use both (as they are unlikely to change much).

pauby avatar Jan 01 '19 16:01 pauby

That's why I suggested using both

that's what I do in my modules :-)

lipkau avatar Jan 01 '19 16:01 lipkau

I would recommend Stroustrup

references: https://en.wikipedia.org/wiki/Indentation_style#Styles https://github.com/PoshCode/PowerShellPracticeAndStyle/issues/81#issuecomment-285835361

lipkau avatar Jan 02 '19 18:01 lipkau

If this can be defined, I can submit a PR by the next day :-)

lipkau avatar Jan 02 '19 18:01 lipkau

sample: https://github.com/lipkau/BuildHelpers/tree/fix/harmonizeCodeStyle

diff: https://github.com/RamblingCookieMonster/BuildHelpers/compare/master...lipkau:fix/harmonizeCodeStyle?expand=1

lipkau avatar Jan 03 '19 10:01 lipkau

Any news on this?

lipkau avatar Apr 26 '19 14:04 lipkau