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

Bhaidar/docs/tutorial #4375

Closed
wants to merge 2 commits into from
Closed

Conversation

bhaidar
Copy link

@bhaidar bhaidar commented Nov 24, 2018

Description:
Migrate old docs/tutorial to the new docs_app/tutorial

Related issue (if exists):

@rxjs-bot
Copy link

Warnings
⚠️

commit message does not follows conventional change log (1)

(1) : RxJS uses conventional change log to generate changelog automatically. It seems some of commit messages are not following those, please check contributing guideline and update commit messages.

Generated by 🚫 dangerJS

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7697

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 96.806%

Totals Coverage Status
Change from base Build 7686: -0.2%
Covered Lines: 5244
Relevant Lines: 5417

💛 - Coveralls

Copy link
Member

@niklas-wortmann niklas-wortmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all thanks a lot for this pr!
I added a bunch of comments but those are very redundant, hope that helps to find all the occurences.

mostly it's usage of var instead of let or const and using old creation functions, e.g. Observable.fromEvent(...) was replaced with fromEvent(...)

If there is anything unclear don't hesitate to ask me.


## Converting to observables

<!-- skip-example -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the skip-example comments were used by the old docs. Nevertheless it don't have an impact in the new docs, therefore I would remove it

import { Observable } from 'rxjs';

// From one or multiple values
const ofObser = Observable.of('foo', 'bar');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you use the static creation operators and the pipable operators in the examples

const ofObser = Observable.of('foo', 'bar');

// From array of values
const fromObser = Observable.from([1, 2, 3]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static from please

const fromObser = Observable.from([1, 2, 3]);

// From an event
const fromEventObser = Observable.fromEvent(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static fromEvent

);

// From a Promise
const fromPromiseObser = Observable.fromPromise(fetch('/users'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fromPromise were deprecated/aliased to from

);

var decreaseButton = document.querySelector('#decrease');
var decrease = Observable.fromEvent(decreaseButton, 'click').pipe(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const and static fromEvent

);

var inputElement = document.querySelector('#input');
var input = Observable.fromEvent(inputElement, 'keypress').pipe(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const and static fromEvent

);

// We merge the three state change producing observables
var state = Observable.merge(increase, decrease, input).pipe(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const + staticmerge function

foo: 'bar'
};

var state = Observable.merge(someObservable, someOtherObservable).pipe(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const + merge


Now you can import your state in whatever UI layer you are using.

<!-- skip-example -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed comment

@benlesh benlesh closed this Nov 28, 2018
@benlesh benlesh reopened this Nov 28, 2018
@benlesh
Copy link
Member

benlesh commented Nov 28, 2018

@bhaidar If you're looking to do this, can you git mv the file so we can see the diff and know if anything has changed?

What was in there definitely needs to be updated, but what is in this PR reads more like a personal blog entry, and seems like it's completely different. I REALLY appreciate the time and thought you put into this, but I'm not sure we can merge this one because it loses context and history, and I can't say I agree with every point in it.

I really think you should take this material and publish it as a blog, and I'd be happy help you refine it and spread it. The RxJS docs are more for approved practices that come from the core team, and this would require a lot of discussion and refinement.

@benlesh benlesh closed this Nov 28, 2018
@niklas-wortmann
Copy link
Member

Sorry for the confusion on this. This content usdd to be in thd old docs and therefore I thought it needed to be migrated. Are there maybe some bits we could use as starting point for the receipt, we planned to have at some point.

I'm really sorry for all the confusion, totally my fault.

@bhaidar
Copy link
Author

bhaidar commented Dec 6, 2018

Hi @benlesh @JWO719
Sorry for being late in getting back to you. I was occupied recently.

I had some issues at first running and updating the docs locally and that explains why most of the changes I've done were lost!

In all cases, I've updated once again the docs.

As Jan said, those are the tutorials from the old docs. I understood from Jan I need to upgrade to RxJs6.

In all cases, I will update the branch and let me know how you would like to proceed.

If there are other things to upgrade from old docs please let me know to work on them.

Best
Bilal

@lock lock bot locked as resolved and limited conversation to collaborators Jan 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants