Skip to content
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

Merge policy for branch 2.x until Release Candidate 1 #4029

Closed
akarnokd opened this issue Jun 18, 2016 · 3 comments
Closed

Merge policy for branch 2.x until Release Candidate 1 #4029

akarnokd opened this issue Jun 18, 2016 · 3 comments

Comments

@akarnokd
Copy link
Member

In #4016, I haven't detailed the merge policy for the 2.0 branch. Here is my proposition:

Until we reach RC 1 (26/8/2016), the merge first, discuss after approach is to become the main theme of branch 2.x.

The general rule is that PRs to be merged have to pass at least the existing unit tests. Please avoid merging PRs that have obviously/deterministically failing tests "to be fixed later".

Terminology

  • Primary API = The base reactive classes (e.g., Flowable, ConnectableX, GroupedX), its base reactive interfaces (e.g., Observer, CompletableSubscriber) and the fusion API elements (e.g., QueueSubscription, ConditionalSubscriber)
  • Secondary API = default public implementations of base interfaces (AsyncSubscriber), Schedulers, etc.
  • Internal API = everything with the package name containing internal

Merges and 👍s

No 👍 needed for:

  • Any pull request that adds or updates functionality: bugfixes, new operators or overloads, performance enhancements, fusion enhancements, adding common or marker interfaces, pulling out common functionality into helper classes.
  • Unit test additions and fixes.
  • PRs deleting internal functionality or methods on internal classes.

I have no strong feelings about marking additions to the public API as @Beta or @Experimental at this stage. I defer to the judgement of the poster of the PR how strong he/she feels about it. (There is going to be an API finalization round/discussion before RC 1).

These rules should help with cases that have triggered lengthy discussions in the past and disagreements about how to proceed. One notorious example is the AsyncOnSubscribe. With these rules, @stealthcode is free to add and merge the operator into 2.x (provided tests still pass). A second example was about PublishProcessor. If I believe it is not mandatory a Processor coordinates downstream and upstream requests, the PR can be merged. In both cases, disagreeing parties may post their own alternative implementations (under a different name) with the expected behavior/API.

Please be considerate with updating someone else's recent addition, especially if it involves changes to its public surface API (if any).

Also please add a reasonable amount unit tests to new operators, overloads and updates (i.e., make sure the operator actually works in the most basic, synchronous scenario). There is going to be a sync with 1.x unit tests before RC 1 and possibly other unit tests based on code coverage tools.

Writing operators/sources in the Reactive-Streams style by itself is more difficult than in 1.x style. I'll post a (wiki?) article about the minimum requirements a 2.x operator has to adhere to.

Writing fusion-enabled operators adds another level of complexity, therefore, it is not required PRs support operator-fusion out of box.

Requires at least 1 👍

  • Deleting elements of the primary API that have been marked @Deprecated in 2.x itself.
  • Deleting elements of the primary API that are @Deprecated, @Beta or @Experimental in 1.x.
  • Deleting from secondary public API
  • Renaming packages, moving things (classes, methods) between internal and public APIs
  • Marking elements @Deprecated, @Beta or @Experimental in 2.x.
  • Disabling or deleting unit tests that are no longer valid/meaningful for 2.x

Requires at least 2 👍

  • Deleting elements from the primary API that have matching established feature(s) (i.e., not marked as @Deprecated, @Beta or @Experimental) in 1.x. The only exception to this rule is the removal of the protected constructors of the base reactive types. These will to be removed due to the architecture change.

Ties or dormant PRs

In case a controversy, lengthy discussion or ties in 👍 and 👎 happen, I reserve the right to be the tiebreaker and make the closing decision.

I consider a PR dormant if the original poster "disappears" and doesn't respond at least for a week. Please don't post a PRs and the go to vacation immediately. PRs that turn out to be dormant may be closed after 10 days by any collaborator.

API finalization

Depending on how fast the changes happen, there is going to be an API finalization round/discussion before RC 1 is released. (Naturally, you don't have to wait till then and have suggestions before that).

The aim is to stabilize the API for RC 1 and allow popular libraries to start developing their own RxJava 2.x based solutions. Depending on this early feedback, RC 2 and RC3 may have further API adjustments.


Let me know if there is any other case to be addressed or something needs further clarification.

@stevegury
Copy link
Member

Sounds good!
As we are in the development mode, I wouldn't bother with annotation (@Beta, @Experimental, ...), we can decide about that during the RC phase.

@zsxwing
Copy link
Member

zsxwing commented Jun 20, 2016

👍

For code coverage, I will submit a PR to use codecov.io like rxjs: https://codecov.io/gh/ReactiveX/rxjs

@akarnokd
Copy link
Member Author

RC1 released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants