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

Remove old docs annotations #7227

Conversation

jakovljevic-mladen
Copy link
Member

Description:
The main purpose of this PR is to remove old docs annotations. E.g.

/**
 * @param {someType} someProp Property description.
 */

From this example, {someType} part is now removed from all @param and @return tag definitions. The docs generator called these types the Catharsis types which may have been required by the old docs, but aren't with the Angular docs.

Also, many other unused tag definitions are removed (like @class or @nocollapse). Plus some more Angular specific stuff.

Many removed items are separated by commits for easier review.

Related issue (if exists):
None.

Copy link
Member Author

@jakovljevic-mladen jakovljevic-mladen left a comment

Choose a reason for hiding this comment

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

@benlesh, I added as much comments as I could find for the easier review, but I also found a couple of other issues that could be fixed, I'll commit them later.

Also, please note that this PR fixes two issues, both of them described in the PR description. And, I really hope this is the last 100+ files changed PR from me 🤣

p.s. We now run docs generation script in the pipelines (as of #6913), therefore we're sure that these changes work and that they don't break the docs generation.

Edit: committed them with this commit.

src/internal/Observable.ts Show resolved Hide resolved
src/internal/observable/bindCallback.ts Show resolved Hide resolved
src/internal/observable/bindCallback.ts Show resolved Hide resolved
* @return {Observable} An Observable of projected values from the most recent
* values from each input Observable, or an array of the most recent values from
* each input Observable.
* @param args Any number of `ObservableInputs`s provided either as an array or as an object
Copy link
Member Author

Choose a reason for hiding this comment

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

There are many APIs that accept ObservableInputs now, but the docs aren't updated. This is only one of such updates where Observable has been replaced with ObservableInputs.

src/internal/observable/defer.ts Show resolved Hide resolved
src/internal/observable/bindNodeCallback.ts Show resolved Hide resolved
src/internal/observable/of.ts Show resolved Hide resolved
* @param count The number of times the source Observable items are repeated, a count of 0 will yield
* an empty Observable.
* @param countOrConfig Either the number of times the source Observable items are repeated
* (a count of 0 will yield an empty Observable) or a {@link RepeatConfig} object.
Copy link
Member Author

Choose a reason for hiding this comment

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

Added RepeatConfig case here.

@@ -58,7 +58,7 @@ import { createOperatorSubscriber } from './OperatorSubscriber';
*
* @param notifier Function that receives an Observable of notifications with which a
* user can `complete` or `error`, aborting the retry.
* @return A function that returns an `ObservableInput` that mirrors the source
* @return A function that returns an Observable that mirrors the source
Copy link
Member Author

Choose a reason for hiding this comment

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

Operators don't return ObservableInputs, just Observables.

* `a.pipe(withLatestFrom(b, c), map(([a1, b1, c1]) => a1 + b1 + c1))`). If this is not
* passed, arrays will be emitted on the output Observable.
* @param inputs An input Observable to combine with the source Observable. More
* than one input Observables may be given as argument. If the last parameter is
Copy link
Member Author

Choose a reason for hiding this comment

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

Tweaked a little since project parameter doesn't exist anymore in the parameter list.

Copy link
Contributor

@demensky demensky left a comment

Choose a reason for hiding this comment

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

Very cool change! I have a couple of questions and comments.

src/internal/Subject.ts Outdated Show resolved Hide resolved
src/internal/ajax/errors.ts Outdated Show resolved Hide resolved
src/internal/observable/forkJoin.ts Outdated Show resolved Hide resolved
src/internal/observable/zip.ts Outdated Show resolved Hide resolved
src/internal/operators/timeout.ts Show resolved Hide resolved
src/internal/util/ArgumentOutOfRangeError.ts Show resolved Hide resolved
src/internal/util/EmptyError.ts Show resolved Hide resolved
src/internal/util/NotFoundError.ts Show resolved Hide resolved
src/internal/util/ObjectUnsubscribedError.ts Outdated Show resolved Hide resolved
src/internal/util/SequenceError.ts Show resolved Hide resolved
Copy link
Member

@benlesh benlesh left a comment

Choose a reason for hiding this comment

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

Spot checked about 90% of the files. Approved out of an abundance of trust.

@jakovljevic-mladen jakovljevic-mladen added the 8.x Issues and PRs for version 8.x label Apr 24, 2023
@jakovljevic-mladen jakovljevic-mladen merged commit 6e3e5e4 into ReactiveX:master Apr 26, 2023
3 checks passed
@jakovljevic-mladen jakovljevic-mladen deleted the remove_old_docs_annotations branch April 26, 2023 13:56
@jakovljevic-mladen
Copy link
Member Author

Approved out of an abundance of trust.

Thanks @benlesh!

Two notes:

  • Changes from this branch will only land to master (for now), therefore, if we spot anything wrong with the content, we can always fix it later (next.rxjs.dev is still not updated with changes from v8).
  • Since changes from this PR are in a lot of ways related to the 7.x branch, I will try to cherry pick them there, but, since these two branches diverged a lot now, I may need to create a new PR for the 7.x branch.

jakovljevic-mladen added a commit that referenced this pull request Apr 27, 2023
* docs: remove owner tag processor

* docs: remove static tag processor

* docs: remove @nocollapse tag definition

* docs: remove @Class tag definition

* docs: remove @constructor tag definition

* docs: remove @method tag definition

* docs: remove @type tag definition

* docs: remove unused tag defs

* docs: remove target-package

* docs: remove more examples package stuff

* docs: remove Catharsis types

* docs: remove Angular specific stuff from docs

* docs: replace "input Observables" with "ObservableInputs"

(cherry picked from commit 6e3e5e4)
@jakovljevic-mladen jakovljevic-mladen added the 7.x Issues and PRs for version 6.x label Apr 27, 2023
@jakovljevic-mladen
Copy link
Member Author

@benlesh, I managed to cherry-pick this commit to 7.x, no changes to the code were made. I added back some docs that were removed from master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.x Issues and PRs for version 6.x 8.x Issues and PRs for version 8.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants