Talk:Style guidelines

Rendered with Parsoid
From Wikitech
Latest comment: 1 year ago by BCornwall in topic Coming to a consensus on Python style guidelines

Coming to a consensus on Python style guidelines

I hear that multiple projects have multiple styles, so which one wins out? I've also heard disagreements on whether to use Black as the style guidelines or to bend Black to whatever we define (I'm of the opinion for the latter) BCornwall (talk) 17:25, 18 August 2022 (UTC)Reply

I also want to clarify that by "wins out" that doesn't exclude the possibility that each team/project has its own style guidelines. Shoehorning something in for the sake of it doesn't help anyone if it causes the project undue stress. I'm more interested in a general "gonna write some code for WMF? Here's how we prefer you do it" --BCornwall (talk) 18:26, 18 August 2022 (UTC)Reply
This is revealing some of my biases and triggers, but the "As a team," starting the article immediately made me say "which team?". The main namespace on wikitech is generally thought of as Wikimedia Foundation SRE oriented content, but even that is not very specific.
I would personally like to think about these kinds of standards and guidelines as FOSS community needs rather than siloed team solutions. mw:Manual:Coding conventions/Python is an existing set of guidelines, but one that would likely be found lacking by a group starting new Python codebase from scratch.
Lacking energy to push for shared standards in the past, the Cloud Services team drafted Wikimedia Cloud Services team/Python coding which can generally be summarized as "use Black with line length 80" (default is 88 chars I believe). The beauty of Black otherwise is that it really isn't variable. Max line length is the only per-project adjustable knob.
Black is only concerned with source code formatting and does not address things like import styles and their sort order, etc. This means that there are certainly still things to debate in building a comprehensive style guide (and things I know that we will never have unanimous consent about).
The Toolhub project I bootstrapped has a style guide, but it is implemented in the configuration of linters including Black rather than by prose description. A convention in that project is that if a linter has not complained about a purely stylistic part of a change the reviewer should not either. Finding a thing in the code that makes your eyes hurt is a prompt to propose a global style change, but not to block a patch. -- BryanDavis (talk) 20:59, 18 August 2022 (UTC)Reply
If the code linter can be considered a Style Guide (I don't agree, a style guide is more than linting and formatting IMHO), then most SRE projects have one. We could try to extract the minimal common denominator between projects. -- Giuseppe Lavagetto (talk) 05:39, 19 August 2022 (UTC)Reply
I agree, Black is a tool to implement the style guidelines and not the guidelines themselves. What happens if Black decides it wants to change a default? BCornwall (talk) 19:38, 19 August 2022 (UTC)Reply
To be completely honest, that paragraph was hashed out without too much thought, so "as a team" (which already kinda sounds like painful business jargon that I've yet to completely purge from my system) can absolutely be clarified. Personally, I don't even care if each team has separate style guides so long as there's documentation that we're all doing our own thing so that we can all abide by expectations by pointing to a document. BCornwall (talk) 19:37, 19 August 2022 (UTC)Reply

Set -u is problematic in many cases and makes shell code unnecessarily complex

We're recommending to use `set -u`, which is a very common recommendation.

I generally don't like it because it eliminates one of the best control flow features of shell, checking if a variable is defined or not. I've found myself multiple times removing `-u` because without it my code was as safe but it was 10% smaller and IMHO more readable.

Given I know this goes against the general wisdom, I'll wait for feedback. -- Giuseppe Lavagetto (talk) 05:43, 19 August 2022 (UTC)Reply

If using bash, one could always expand the variable to ${myvar:-unset}. I don't think it's safe to omit -u considering the potential for damage something like rm has when sent a blank variable. --BCornwall (talk) 19:34, 19 August 2022 (UTC)Reply