Page MenuHomePhabricator

Policy for (external) contributions / code style
Open, MediumPublic

Description

General problem statement:
As a tool maintainer and contributor to Toolforge features having a clear policy document/guidance covering how contributions should be raised and the general expectations for their contents would significantly reduce friction.

Background:
Having used WMCS since ~2013 and Toolforge since ~2015 lots of changes have happened, including the introduction of k8s (from grid engine) and the now 'standard' PaaS type APIs.

After the declaration in 2024 that handling k8s objects directly was un-supported, efforts where made to establish what was missing from the "supported" interfaces in order to continue running the production workloads, keeping in mind both that editors are reasonably sensitive to downtime and new (brought back to life) features where planned.

This has had mixed success, with everything being incrementally migrated to run from builds-api images and as of last week under components. Many compromises have been accepted (outage for every deployment) with a view that the changes could be landed to improve the behaviour (for all, infrastructure resources included).

Unfortunately the contribution experience has at times not been very inspiring.

Examples of feedback:

Note: I will refrain from linking to the direct instances as not to single anyone in particular out

there is something about putting more things in the label that doesn't seem right to me. Right now my preference is to remove as much things as we can, not add them

This is completely subjective and to me not inline with https://www.mediawiki.org/wiki/Wikimedia_Cloud_Services_team/Team_Social_Norms#Propose_a_different_path_if_you_oppose_one.

Simply re-framing this as "we have seen maintainability issues with labels (Pxxx) and prefer to limit their usage, can you change this to use annotations" would have provided a clear reference to context and a path forwards.

Developing to someone's feelings or preference is essentially playing guess work.

unnecessary change detected. both effectively mean the same thing so why introduce the diff

The code style documents linked (see below) suggest to fix existing code if low effort to follow PEP 8, generally it's reasonable to cleanup code within a function that is being changed (as the entire function needs reviewing anyway).

If this is the policy then it should be clearly documented and the comment can be a reference to the policy for consistency.

While perhaps not intended, I personally find the wording here quite rude, almost hostile.

Keeping the above in mind, all these diffs do nothing in particular for us. We don't want it to handle general cases where urls are given, and we only want to add 1 or 2 lines to extract digest.

While perhaps not intended "all these diffs do nothing in particular for us" reads directly in conflict with https://www.mediawiki.org/wiki/Wikimedia_Cloud_Services_team/Team_Social_Norms#Support_other_people_&_our_work

Outside if there being enough context and issue linkage to answer the question being asked, it could very easily be re-worded to not sound like it's a complete waste of time while ignoring the problem it solves.

Having clear documentation of the "we want"'s would make it clear what the desires/expectations are upfront and save the interaction, rather than guessing how a reviewer may feel on the day.

Other items to consider in a policy / general expectations:

  1. Workflows - it was requested/assumed labels can be added to MRs, this is not possible for non-maintainers, requiring a "sponsor" to get a MR into the review queue
  2. Tooling - toolforge_deploy_mr.py (as an example) was a good addition, but never worked for non-maintainers. It would have been very easy to test this/changes to the pipeline as a non-maintainer, rather than offloading the effort onto volunteers.

What exists today (at least what I could find):
https://www.mediawiki.org/wiki/Wikimedia_Cloud_Services_team/Decision_Making
https://www.mediawiki.org/wiki/Wikimedia_Cloud_Services_team#Objectives
https://www.mediawiki.org/wiki/Wikimedia_Cloud_Services_team/Team_Social_Norms
https://wikitech.wikimedia.org/wiki/Wikimedia_Cloud_Services_team/Coding
https://www.mediawiki.org/wiki/Manual:Coding_conventions/Python

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Some other related tasks about coding standards (and discussions that followed): T327087: Decision request: python source code line length T361804: Decision request - Update python team best practices

It should be noted that agreeing in a coding standard has been a very contentious discussion in the past, though we might want to give it another shot as it clearly helps newcomers and external contributors avoid getting long reviews.