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

API doc cleanup for decorators #31377

Closed
wants to merge 2 commits into from

Conversation

@jbogarthyde
Copy link
Contributor

commented Jul 1, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Makes descriptions parallel, cleans up styles and formats to be consistent.
Mostly for decorators, but some other API issues that were reported.

Does this PR introduce a breaking change?

  • Yes
  • No
@mary-poppins

This comment has been minimized.

Copy link

commented Jul 1, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 1, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 1, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 1, 2019

@IgorMinar
Copy link
Member

left a comment

great improvements! thank you

I left a few comments / suggestions.

Show resolved Hide resolved packages/core/src/di/interface/provider.ts Outdated
Show resolved Hide resolved packages/core/src/di/interface/provider.ts
* ### Example
*
* {@example core/di/ts/provider_spec.ts region='ExistingSansProvider'}
* {@example core/di/ts/provider_spec.ts region='ExistingSansProvider' linenums="false"}

This comment has been minimized.

Copy link
@IgorMinar

IgorMinar Jul 1, 2019

Member

does this region actually exist? I don't see it defined in the provider_spec.ts file.

I just noticed that ConstructorProvider is not exported from @angular/core at all (this makes it inaccessible to developers for direct interaction but allows type inference and typechecking to work just fine - meaning that developers would usually not complain about the lack of exporting but the docs for this symbol are missing on angular.io)

can you please fix this my exporting the symbol from packages/core/src/di/index.t and update tools/public_api_guard/core/core.d.ts to reflect the adition?

This comment has been minimized.

Copy link
@jbogarthyde

jbogarthyde Jul 2, 2019

Author Contributor

I can update the index, but I don't have access to the golden file. I've asked George to do it.

This comment has been minimized.

Copy link
@gkalpak

gkalpak Jul 2, 2019

Member

I foudn out there were a few more missing, so added them all (on a separate commit): 567e92b

Show resolved Hide resolved packages/core/src/linker/component_factory_resolver.ts Outdated

@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jul 2, 2019

@jbogarthyde jbogarthyde force-pushed the jbogarthyde:jb-apidoc-decorators branch 2 times, most recently from 01ddfc4 to 967f440 Jul 2, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 2, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 2, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 2, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 2, 2019

@gkalpak gkalpak force-pushed the jbogarthyde:jb-apidoc-decorators branch from c3dd1ff to 0db38da Jul 2, 2019

@jbogarthyde jbogarthyde requested a review from angular/fw-public-api as a code owner Jul 2, 2019

@gkalpak gkalpak force-pushed the jbogarthyde:jb-apidoc-decorators branch from 0db38da to d457932 Jul 2, 2019

gkalpak added a commit to jbogarthyde/angular that referenced this pull request Jul 2, 2019

fix(core): export provider interfaces that are part of the public API…
… types

Some of the provider interfaces that the [Provider][1] and
[StaticProvider][2] types comprise were not exported from
[@angular/core][3]. As a result, the docs for these symbols did not
appear on angular.io (even though both `Provider` and `StaticProvider`
are part of the public API. (See, also,
angular#31377 (comment).)

This commit fixes it by exporting all necessary provider interfaces.

[1]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L365-L366
[2]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L283-L284
[3]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/index.ts#L23
@mary-poppins

This comment has been minimized.

Copy link

commented Jul 2, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 2, 2019

@ngbot

This comment has been minimized.

Copy link

commented Jul 3, 2019

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "cla/google" is failing
    failure status "ci-codefresh-bazel" is failing
    pending missing required labels: cla: yes
    pending forbidden labels detected: cla: no
    pending status "google3" is pending
    pending 1 pending code review

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@jbogarthyde

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

Please fix the CLA and re-run codefresh.

@jbogarthyde jbogarthyde force-pushed the jbogarthyde:jb-apidoc-decorators branch from f1e57b8 to 4a76e3c Jul 3, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 3, 2019

@jbogarthyde jbogarthyde force-pushed the jbogarthyde:jb-apidoc-decorators branch from 4a76e3c to 3e38ced Jul 3, 2019

jbogarthyde added a commit to jbogarthyde/angular that referenced this pull request Jul 3, 2019

fix(core): export provider interfaces that are part of the public API…
… types

Some of the provider interfaces that the [Provider][1] and
[StaticProvider][2] types comprise were not exported from
[@angular/core][3]. As a result, the docs for these symbols did not
appear on angular.io (even though both `Provider` and `StaticProvider`
are part of the public API. (See, also,
angular#31377 (comment).)

This commit fixes it by exporting all necessary provider interfaces.

[1]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L365-L366
[2]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L283-L284
[3]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/index.ts#L23
@mary-poppins

This comment has been minimized.

Copy link

commented Jul 3, 2019

@jbogarthyde jbogarthyde force-pushed the jbogarthyde:jb-apidoc-decorators branch from 3e38ced to e6a1678 Jul 3, 2019

jbogarthyde added a commit to jbogarthyde/angular that referenced this pull request Jul 3, 2019

fix(core): export provider interfaces that are part of the public API…
… types

Some of the provider interfaces that the [Provider][1] and
[StaticProvider][2] types comprise were not exported from
[@angular/core][3]. As a result, the docs for these symbols did not
appear on angular.io (even though both `Provider` and `StaticProvider`
are part of the public API. (See, also,
angular#31377 (comment).)

This commit fixes it by exporting all necessary provider interfaces.

[1]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L365-L366
[2]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L283-L284
[3]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/index.ts#L23
@mary-poppins

This comment has been minimized.

Copy link

commented Jul 3, 2019

gkalpak and others added some commits Jul 2, 2019

fix(core): export provider interfaces that are part of the public API…
… types

Some of the provider interfaces that the [Provider][1] and
[StaticProvider][2] types comprise were not exported from
[@angular/core][3]. As a result, the docs for these symbols did not
appear on angular.io (even though both `Provider` and `StaticProvider`
are part of the public API. (See, also,
#31377 (comment).)

This commit fixes it by exporting all necessary provider interfaces.

[1]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L365-L366
[2]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L283-L284
[3]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/index.ts#L23

@jbogarthyde jbogarthyde force-pushed the jbogarthyde:jb-apidoc-decorators branch from e6a1678 to 742b170 Jul 8, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented Jul 8, 2019

@jasonaden jasonaden added cla: yes and removed cla: no labels Jul 8, 2019

@googlebot

This comment has been minimized.

Copy link

commented Jul 8, 2019

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@jasonaden jasonaden closed this in 35f8bfc Jul 8, 2019

jasonaden added a commit that referenced this pull request Jul 8, 2019

jasonaden added a commit that referenced this pull request Jul 8, 2019

fix(core): export provider interfaces that are part of the public API…
… types (#31377)

Some of the provider interfaces that the [Provider][1] and
[StaticProvider][2] types comprise were not exported from
[@angular/core][3]. As a result, the docs for these symbols did not
appear on angular.io (even though both `Provider` and `StaticProvider`
are part of the public API. (See, also,
#31377 (comment).)

This commit fixes it by exporting all necessary provider interfaces.

[1]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L365-L366
[2]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/interface/provider.ts#L283-L284
[3]: https://github.com/angular/angular/blob/9e34670b2/packages/core/src/di/index.ts#L23

PR Close #31377

jasonaden added a commit that referenced this pull request Jul 8, 2019

@jbogarthyde jbogarthyde deleted the jbogarthyde:jb-apidoc-decorators branch Jul 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.