-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Experimental CO proposal #10
Open
acolombier
wants to merge
1
commit into
mixxxdj:main
Choose a base branch
from
acolombier:experimental-co
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
# Experimental CO API Prefixes | ||
|
||
* **Owners:** | ||
* `@acolombier` | ||
|
||
* **Implementation Status:** `Implemented` | ||
|
||
* **Related Issues and PRs:** | ||
* #13268 | ||
* #13300 | ||
|
||
* **Other docs or links:** | ||
* `<Links…>` | ||
|
||
> TL;DR: Introduce a new standard prefix for CO that raises concerns when submitted. | ||
|
||
## Why | ||
|
||
As we've recently seen in a few PRs, it happens that suggested CO values or names raise concerns about whether they are | ||
perfectly fit for purpose. That is, whether their name is 100% clear to our users and whether the value representation | ||
is usable. | ||
|
||
### Pitfalls of the current solution | ||
|
||
Currently, our way to address this issue is to get all reviewers involved to agree on what's proposed in a PR. This | ||
often leads to a "too many cooks" problem and an oracle game, where everyone tries to predict the future evolution of | ||
Mixxx. As we all know, the future rarely | ||
happens as expected, and limitations or design decisions turn out to be outdated after a certain time. | ||
|
||
## Goals | ||
|
||
The goal of this proposal is to define a process that allows quick iteration on API changes while ensuring the | ||
stability that the Mixxx Development team has fought so hard to keep intact. A requirement for the outcome of this | ||
proposal is to require a minimum of code changes, | ||
ideally none, and focus on establishing a process that contributors can easily align with. | ||
|
||
### Audience | ||
|
||
The audience of this proposal is strictly targeted at Mixxx core contributors, who may be writing C++ code to the | ||
Mixxx codebase. | ||
|
||
## Non-Goals | ||
|
||
* Introduce core API code changes | ||
* Allow any CO proposition to be accepted. If there's clear consensus against a CO proposition, it won't be | ||
eligible for the experimental process | ||
|
||
## How | ||
|
||
To allow quick changes, COs that don't get consensus at submission can be submitted as an experimental CO. An | ||
experimental CO will always be prefixed with the [experimental marker](#experimental-marker). Typically, | ||
experimentation will never last more than a version (~ 6 months), but the Mixxx Core Team may decide to extend the | ||
experimentation by a second version cycle if there's not enough feedback during the first cycle or if there remain | ||
risks for negative user feedback or lacking use case coverage to confirm that the CO is mature enough to exit the | ||
experimentation sandbox. During the version following the CO stable promotion, an alias should be kept to its former | ||
key to prevent breaking changes. The Mixxx Core Team may decide to extend the alias period by a second version cycle | ||
but should refrain from doing it more than once in order to reduce the amount of COs that have an impact on | ||
performance. | ||
|
||
### Experimental marker | ||
|
||
The suggested experimental marker is the prefix `x_`, e.g., `x_beats_set_change_marker`. | ||
Comment on lines
+60
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unsure whether this experimental marker should be naming related. An explicit opt-in may be better (add a flag for creating the CO and only make it accessible if the consumer explicitly opts in). That would require some slight API changes though... |
||
|
||
### Process | ||
|
||
The suggested process to ensure proper follow-up of an experimental CO could be the following: | ||
|
||
* An issue should be created using a standard template. This issue will remain open during the experimentation period | ||
and target the milestone `<version>+1`. | ||
* Documentation of the CO should include a note about the experimental aspect of that CO. | ||
* At `<version>+1`, the CO is trimmed from its experimental prefix, and the documentation is updated. | ||
* At `<version>+2`, the CO experimental alias is dropped. | ||
|
||
### Standard template | ||
|
||
```plaintext | ||
Subject: Experimental CO: <StableCOName> | ||
|
||
Body: | ||
|
||
Introduced by: #<PRNumber> | ||
Documented by: #<ManualPRNumber> | ||
Usecase: | ||
<Short summary of the scope> | ||
``` | ||
|
||
Here's an example of issue: | ||
|
||
```plaintext | ||
Subject: Experimental CO: beats_set_change_marker | ||
|
||
Body: | ||
|
||
Introduced by: #13300 | ||
Documented by: #702 | ||
Usecase: | ||
Sets a new independent marker grid at the current play position. The new marker grid will inherit the BPM from its | ||
predecessor but will be fully autonomous | ||
``` | ||
|
||
*Note that `Usecase` will largely be inspired of the manual, but may also capture technical implementation details that | ||
may help assessing the stability of the CO during its probation* | ||
|
||
### Completion | ||
|
||
During the experimentation period, one may suggest dropping a CO for the following reason: | ||
|
||
* Bug report due to API complexity or lack of use case support | ||
* Superseding of behavior by newly introduced CO | ||
|
||
Once the probation period terminates, the CO should be considered stable, except if an extension is asked by any | ||
member of the core team. | ||
|
||
## Alternatives | ||
|
||
No alternatives is currently being considered. | ||
|
||
## Action Plan | ||
|
||
The tasks to do in order to migrate to the new idea. | ||
|
||
* Standard template to be captured in the Wiki or Issue template |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd drop that IMO. These experimental COs are strictly for experimentation. Promising any sort of stability guarantees makes it too easy to get distracted from that experimentation by trying to uphold those guarantees. An experimental CO should only be used by in-tree code, that way removing code relying on it is as easy as
grep
ping for it.