RamblingCookieMonster / BuildHelpers

Helper functions for PowerShell CI/CD scenarios
MIT License
212 stars 47 forks source link

Inconsistent code style #74

Open lipkau opened 5 years ago

lipkau commented 5 years ago

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?

RamblingCookieMonster commented 5 years ago

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.).

pauby commented 5 years ago

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

lipkau commented 5 years ago

Not really. Primary advantages are:

pauby commented 5 years ago

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).

lipkau commented 5 years ago

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

pauby commented 5 years ago

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).

lipkau commented 5 years ago

That's why I suggested using both

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

lipkau commented 5 years ago

I would recommend Stroustrup

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

lipkau commented 5 years ago

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

lipkau commented 5 years ago

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

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

lipkau commented 5 years ago

Any news on this?