Skip to content

chore(docs): add types to public and jsdoc cleanup #7711

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Jan 22, 2018
Merged

chore(docs): add types to public and jsdoc cleanup #7711

merged 11 commits into from
Jan 22, 2018

Conversation

amcdnl
Copy link
Contributor

@amcdnl amcdnl commented Oct 11, 2017

This PR:

  • Cleans up missing types on PUBLIC methods/properties
  • Cleans up JSDoc comments
  • Adds more excludes for internal methods

@amcdnl amcdnl self-assigned this Oct 11, 2017
@amcdnl amcdnl requested a review from jelbourn October 11, 2017 14:16
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 11, 2017
jelbourn
jelbourn reviewed Oct 11, 2017
View reviewed changes
tools/dgeni/processors/docs-private-filter.ts Outdated
'setDescribedByIds',
'registerOnValidatorChange',
'onContainerClick',
'shouldPlaceholderFloat',
Copy link
Member

Choose a reason for hiding this comment

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

Why add these here instead of marking them with @docs-private?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was on the fence either way. The pattern seem to be doing this over @docs-private so I went this way. Plus this is on several different components so easier to do it in one place vs several.

Copy link
Contributor

Choose a reason for hiding this comment

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

setDescribedByIds, onContainerClick and shouldPlaceholderFloat are all part of MatFormFieldControl which I do want to have an API doc, I think doing it this way might inadvertently hide them there too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mmalerba - Got it, I'll pull them out in that case.

amcdnl added 2 commits October 16, 2017 10:43
@amcdnl
Merge branch 'master' of https://github.com/angular/material2 into do…
4821f5e 
…cs-cleanups

# Conflicts:
#	src/lib/expansion/accordion.ts
#	src/lib/expansion/expansion-panel.ts
#	src/lib/list/selection-list.ts
@amcdnl
chore(nit): remove global ignore in favor of specific
8c561d1
jelbourn
jelbourn approved these changes Oct 27, 2017
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Oct 27, 2017
@amcdnl amcdnl mentioned this pull request Nov 5, 2017
Closed
@josephperrott
Copy link
Member

@amcdnl Needs rebase.

@amcdnl
Copy link
Contributor Author

amcdnl commented Nov 19, 2017

@josephperrott done :)

@mmalerba
Copy link
Contributor

needs rebase again

@amcdnl
Copy link
Contributor Author

amcdnl commented Dec 2, 2017

@mmalerba - done :)

@mmalerba
Copy link
Contributor

mmalerba commented Dec 6, 2017

needs rebase again

@amcdnl
Copy link
Contributor Author

amcdnl commented Dec 10, 2017 •
edited
Loading

@mmalerba - Done. Can we get this merged soon, there is no code changes just doc updates.

@mmalerba mmalerba added target: patch This PR is targeted for the next patch release and removed pr: needs rebase labels Dec 11, 2017
@andrewseguin
Copy link
Contributor

Please check CI issues (incl lint)

@andrewseguin andrewseguin added presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged and removed presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged labels Dec 13, 2017
@josephperrott
Copy link
Member

needs rebase again

@josephperrott josephperrott added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Jan 19, 2018
@amcdnl
Copy link
Contributor Author

amcdnl commented Jan 21, 2018

@josephperrott - done :)

@amcdnl
Copy link
Contributor Author

amcdnl commented Jan 21, 2018

@josephperrott bump

@jelbourn jelbourn added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels Jan 22, 2018
jelbourn
jelbourn approved these changes Jan 22, 2018
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn merged commit b071d64 into angular:master Jan 22, 2018
@rafaelss95 rafaelss95 mentioned this pull request Jan 24, 2018
Closed
@amcdnl amcdnl deleted the docs-cleanups branch January 26, 2018 13:07
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mmalerba mmalerba mmalerba left review comments

@jelbourn jelbourn jelbourn approved these changes

Assignees

@amcdnl amcdnl

Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@amcdnl @josephperrott @mmalerba @andrewseguin @jelbourn @googlebot