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

docs(general): clarify introduction section #5419

Closed

Conversation

niklas-wortmann
Copy link
Member

@niklas-wortmann niklas-wortmann commented May 8, 2020

I started to go over the general information section to remove some technical bla bla. It aims to make it easier for the reader to understand the important concepts and focus on the relevant parts!

Additionally, I slightly changed the sidebar and split the guides in an "Essential" and an "Advanced" section. We could add a small paragraph somewhere describing this structure to give an overview, but I think for now I can imagine this to help people to get into this by focusing on the essentials. At the same time, I split some larger sections into multiple smaller one and put them in the related category. E.g. the former operators guide is split into operators, marble diagrams & higher-order Observables

@@ -22,7 +22,7 @@ To import only what you need using pipeable operators:
import { of } from 'rxjs';
import { map } from 'rxjs/operators';

of(1,2,3).pipe(map(x => x + '!!!')); // etc
of(1,2,3).pipe(map(x => x + '!!!'));
Copy link
Member

Choose a reason for hiding this comment

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

Hi there. There is an empty space left after ; which might be removed. The other one couple of lines below is removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for spotting this


What makes RxJS powerful is its ability to produce values using pure functions. That means your code is less prone to errors.
This is only an example demonstrating the behavior of reactive programming.
Copy link
Member

Choose a reason for hiding this comment

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

This file has also couple of empty spaces at the end of some lines. If this is not important, please let me know, I won't report it anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have really strong opinions about that. IMO it helps maintaining the markdown file

Copy link
Member

Choose a reason for hiding this comment

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

I kinda don't like empty spaces at the end of lines, they look a bit dirty to me. But if they can help, sure, keep having them.

@@ -29,13 +29,13 @@ const observer = {
};
```

When subscribing to an Observable, you may also just provide the callbacks as arguments, without being attached to an Observer object, for instance like this:
When subscribing to an Observable, you can also provide the callbacks as anonymous function, without wrapping it in an object:
Copy link
Contributor

@joshribakoff joshribakoff Jun 17, 2020

Choose a reason for hiding this comment

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

Whether or not you pass an anonymous function to the argument, does not matter, i don't think. https://stackblitz.com/edit/rxjs-ftnxyh

Could we consider rewording to "you can pass references to callback functions as the arguments"?

Copy link
Member Author

Choose a reason for hiding this comment

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

good point. Thanks a lot for your input on that. Happy to use this wording

@benlesh
Copy link
Member

benlesh commented Sep 2, 2020

@niklas-wortmann we need to think of a way to merge this work with the work I'm doing over on #5592

@benlesh benlesh added the AGENDA ITEM Flagged for discussion at core team meetings label Sep 2, 2020
@niklas-wortmann niklas-wortmann changed the base branch from master to 6.x September 30, 2020 14:42
@niklas-wortmann
Copy link
Member Author

niklas-wortmann commented Sep 30, 2020

@benlesh as #5592 is going in master and will therefore be the updated docs for v7 and upcoming I would suggest moving this to the v6 branch to add some clarifications there. Now that I see the merge conflicts I will probably create a new pr branching of v6. I will just keep this open in the meantime to have the changes visible :D

@niklas-wortmann niklas-wortmann changed the base branch from 6.x to master September 30, 2020 14:46
@benlesh
Copy link
Member

benlesh commented Oct 7, 2020

Core Team: this goes to v6 docs.

@benlesh benlesh removed the AGENDA ITEM Flagged for discussion at core team meetings label Oct 7, 2020
@benlesh
Copy link
Member

benlesh commented Oct 13, 2020

Looks like this needs rebased.

@niklas-wortmann
Copy link
Member Author

closed by #5871

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

Successfully merging this pull request may close these issues.

4 participants