Page MenuHomePhabricator

Lower the burden of contributing code to Codex
Closed, InvalidPublic

Description

Background

Summary

Because Codex is a shared library that must be maintainable, meet stringent accessibility/usability/design standards, and be understandable to users of the library, the burden of contributing is already high. Additionally, getting started with Codex development requires learning skills that aren't required for MediaWiki development, or at least haven't been required in the past (TypeScript, Vue 3/composition API, etc.) and learning a bespoke component documentation platform. We should consider how we can lower the burden for new contributors.

Impact

As a potential code contributor to Codex, I want to understand the various ways I could contribute code and what level of difficulty/time commitment is involved in each of those pathways.

As a new code contribute to Codex, I want to be able to make a satisfying and meaningful impact, without being bogged down by frustration

As a new code contributor to Codex, I want to be able to quickly contribute, without having to spend a large amount of time reading documentation or learning new technologies.

As a maintainer of Codex, I want to enable contributions from others, so we can benefit from their expertise and ideas and so that more work can be done on Codex than just the core maintainers could do on their own.

Lowering the burden of contributing code to Codex would have impact in several ways:

  • Potential contributors will easily understand how they can contribute, whether that's a small patch with a simple fix or a new test, a new feature added to an existing component, or an entirely new component. Potential contributors with varying levels of time/availability and experience with technologies used within Codex will be able to find a way to meaningfully contribute.
  • New contributors will be able to make a meaningful impact more quickly and with less frustration
  • Codex maintainers will benefit from the work and ideas of people outside of the Design Systems Team
  • More people will be familiar with and bought into Codex, increasing adoption and support for the project
  • More work will get done on Codex, which will also lead to increased adoption and furthering the goals of the library at large (consistent UX and design, faster designer and developer velocity, wider use of modern technologies in the wikiverse, etc)

Proposed solutions

Removing demo requirements

Status: Complete – see T314792 and T311243

One requirement we could drop is the requirement of building component demos in VitePress. VitePress is not widely used and our setup is mostly custom, so it requires some explanation and will be new to all new contributors. Instead, we could allow demos to be created in separate tasks, and suggest that developers use the sandbox that comes with Vite in the Codex package.

This would require:

  • Updating the contributing code docs to remove the requirement of VitePress demos and, instead, state that they can either be created during component development or separately by another developer.
  • Clean up the sandbox, which is getting to be disorganized and unwieldy since it is a single file containing all components (see T311243)

Add section for experimental components

Status: Complete. See T313767

Another pathway we've considered in the past (see patch) is creating a separate entry for "experimental" components that aren't ready for wider use. We could set different requirements for components in this category and even forego testing them in CI.

Document the various pathways to code contribution

Status: Complete. See T313768

Currently, the contributing code docs contain an extensive list of patch requirements for new components. By implementing some of the other solutions above, we could revise this to explain that there are various ways to contribute code:

  • The current process where the component, full set of demos, and full test coverage are completed
  • An incremental process where the component, demos, and tests could be added separately (either by a single developer or multiple)
  • A partnership process where a contributor could create the component or new feature, and partner with a DST member who will write tests and/or demos in related patches at the same time
  • Probably more!

Support local development within Fresh environment

Status: To do. See T309899

Many MediaWiki developers use Fresh to run Node.js in a Docker environment for local development. We should ensure that this local dev pathway is supported and covered in our developer docs.

Implement VueUse for pre-made composition utilities

Status: To do. See T307089

VueUse is a library of composition utilities, or composables, that provide commonly needed functionality within Vue components. We currently write our own composables from scratch, but we could make component development easier and more standard by adopting VueUse and using the pre-made composables whenever possible. This would prevent developers from having to write the functionality themselves, and would make our work more standardized and therefore easier to onboard to.

Set clearer coding conventions

Status: To do. See T308520, T299242 , and T298667

There are a few areas where we could be more clear about the conventions we follow when we write code. Standardizing and documenting these conventions takes the guesswork out of local development, especially for a first-time Codex code author. We have 3 open tasks related to standards we need to determine and document:

  • T308520: Standards around unit test naming and organization
  • T299242: Conventions for custom event names
  • T298667: Conventions for demo code

Set clear code review processes and guidelines

Status: Complete. See T313765

Code review can be a stressful, time-consuming, and confusing process. Let's make our processes very clear so contributors know what to expect, and set guidelines for healthy code review conduct to ensure that contributors are welcomed and encouraged.

Make the code review process easier through automation

Status: To do. See T313540

We already make use of things like linters to automatically catch common coding errors and issues. The more we can catch through these automated tools, the clearer the resolutions are to the code author, and the less time we have to spend discussion those issues during code review. Let's consider what else we can automate away.

Removing or changing unit test requirements

Status: Declined per discussion in DST engineering meeting 8/8/22 – unit test requirements should remain in place

Related Objects

Event Timeline

@AnneT and I were discussing whether a different development model for Codex would help lower barriers to contribution from other developers. Right now, the expectation is that every new component patch must contain a ton of sub-tasks: updates to tests, docs/demo pages, etc. All of this work must be handled as part of a single patch and nothing gets merged until it is all complete and as close to perfect as possible.

I wonder if this way of working is making things more difficult for us than they need to be? Since Codex is an external library, we don't need the main branch to be stable at all times. We cut dedicated releases at various points and periodically update the copy that is bundled into MediaWiki to the latest release.

We could move to a development model where main is understood to be in an unstable, work-in-progress state. A single patch could introduce a new component, another one could add the demos, another one could update tests or docs, etc. This might allow us to break up work more efficiently, and to commit smaller patches (easier for review).

removing the demo requirement is one part of this, but I think the bigger question is whether we would be okay with a different development style in Codex.

AnneT renamed this task from Make local development of components easier by removing the demo requirement to Make local development of components easier by limiting patch requirements.Jul 21 2022, 6:06 PM
AnneT updated the task description. (Show Details)

Re unit test coverage, one way we could do this is define thresholds on a per-file basis instead of globally. Alternatively, if we create the concept of an experimental component and put those components in a separate directory, we could have per-directory thresholds.

AnneT renamed this task from Make local development of components easier by limiting patch requirements to [EPIC} Make local development of components easier by limiting patch requirements.Jul 25 2022, 8:53 PM
AnneT renamed this task from [EPIC} Make local development of components easier by limiting patch requirements to [EPIC] Make local development of components easier by limiting patch requirements.
AnneT added a project: Epic.
AnneT updated the task description. (Show Details)
AnneT updated the task description. (Show Details)
AnneT renamed this task from [EPIC] Make local development of components easier by limiting patch requirements to [EPIC] Lower the burden of contributing code to Codex.Aug 1 2022, 8:52 PM
AnneT claimed this task.
AnneT updated the task description. (Show Details)

Thanks @AnneT for the update. It seems like we could prioritize Removing demo requirements", since we've already started to action some of this work via maybe try to close out some of the demo site related work "Removing demo requirements" via https://phabricator.wikimedia.org/T311243. Would the only remaining task to add be that we need to spin out a sub-task related to updated demo requirements documentation?

@AnneT Some additional thoughts:

  • For Document the various pathways to code contribution section:
    • Perhaps we add these notes to https://phabricator.wikimedia.org/T313940. I think this is probably deserving of its own task and we can either refine the requirements there or spin out additional sub-tasks as needed.
      • Could we also outline what we already have from these recommendations on that ticket?
  • For Removing or changing unit test requirements section:
    • When we want to merge a patch, could we also just create a workflow that lets us make a merge request, which gets queued up and then can be picked up by someone to incorporate tests? Is there a reason we would patch separately? (Sorry I'm not super familiar with our CI process...

@DAbad I meant for T313768 to cover all of the documentation updates needed for this task, including removing the demo requirement and documenting the various pathways to code contribution, but we can split it out into multiple tasks if needed.

Prioritizing removing the demo requirements makes sense to me; once the Vite sandbox task is complete, it'll just be a matter of updating the docs as you said.

  • For Removing or changing unit test requirements section:
    • When we want to merge a patch, could we also just create a workflow that lets us make a merge request, which gets queued up and then can be picked up by someone to incorporate tests? Is there a reason we would patch separately? (Sorry I'm not super familiar with our CI process...

We could definitely do this, at least as a first step until we're able to prioritize T313767. The downsides to this would be:

  • The patch would need to stay open longer. When a patch stays open for more than a few days, it must be regularly synced with the main branch, which can be tedious
  • Because of that, we'd want to assign someone to complete the tests quickly, so we wouldn't have quite as much flexibility on timeline as we would if we could merge the initial patch and let it sit in the experimental components bin until we could get around to completing it
  • We usually try to break work into separate patches to keep each patch more focused and easier to review (although we currently include unit tests in patches, so this isn't a huge deal in this case)

Still, I think this is a good idea that would allow us to update our processes immediately.

DAbad changed the subtype of this task from "Task" to "Spike".

As a note scoping tickets should not be actioned directly

DAbad renamed this task from [EPIC] Lower the burden of contributing code to Codex to Lower the burden of contributing code to Codex.Aug 8 2022, 5:08 PM
DAbad changed the task status from Open to In Progress.
egardner triaged this task as Medium priority.Aug 9 2022, 12:30 AM

Since we are now considering this a scoping ticket (essentially a spike) instead of an epic, I have removed the previous sub-tasks (and attached them to the most appropriate-seeming parents).

The guidelines being drafted here can inform those other tasks; this task can be closed (but still used as a reference) when we feel that it has been sufficiently elaborated.

I'm not sure if this is the best place to give this feedback, but I think it falls under lowering the burden for contribution.

While working on T306573: Copy button in Codex demos copies extra newline in Chrome I was not able to set the IP and port that the server should run on when using npm run doc or npm run doc:dev. I had to modify the vite.config.ts file in order to change these configurations.

Some background: I do my development inside a virtual machine. To access the running Node.js server from the host machine, I need to access it using the VM's IP address.

Change 823262 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] build: Add "style" field to package.json

https://gerrit.wikimedia.org/r/823262

Change 823262 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] build: Add "style" field to package.json

https://gerrit.wikimedia.org/r/823262

Sorry for the noise, this change was tagged with the wrong task.

Change 823266 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] build: Make Vite port configurable, listen on all IPs

https://gerrit.wikimedia.org/r/823266

I'm not sure if this is the best place to give this feedback, but I think it falls under lowering the burden for contribution.

While working on T306573: Copy button in Codex demos copies extra newline in Chrome I was not able to set the IP and port that the server should run on when using npm run doc or npm run doc:dev. I had to modify the vite.config.ts file in order to change these configurations.

Some background: I do my development inside a virtual machine. To access the running Node.js server from the host machine, I need to access it using the VM's IP address.

Thanks for pointing out this problem! I hope the attached patch solves this issue. It sets host to 0.0.0.0, and allows you to override the port with npm run doc:dev --port=12345.

Change 823266 merged by jenkins-bot:

[design/codex@main] build: Make Vite port configurable, listen on all IPs

https://gerrit.wikimedia.org/r/823266

Change 823725 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[mediawiki/core@master] Update Codex from v0.1.0-alpha.9 to v0.1.0-alpha.10

https://gerrit.wikimedia.org/r/823725

Change 823725 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.1.0-alpha.9 to v0.1.0-alpha.10

https://gerrit.wikimedia.org/r/823725

Thanks @DannyS712; I've added that to the list.

I've also added a number of other existing tasks that would contribute to the goal of lowering the burden of code contribution.

ldelench_wmf changed the subtype of this task from "Spike" to "Task".Sep 1 2022, 11:50 AM
ldelench_wmf subscribed.

Discussed with @AnneT : we'll convert this scoping spike into our delivery epic, move it to the roadmap board, and attach the tasks listed in the description as subtasks

@ldelench_wmf this shouldn't be in Needs Triage, but I'm not sure where it should be. What do you think?

The Design Systems team is removing the majority of our "tracking" tasks like this one.
Subtasks have been moved to the Contributor Experience column of the Codex workboard.