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

[all] updating components with tsdocs comments to properties, additio… #125

Merged
merged 14 commits into from
Aug 28, 2019

Conversation

vogtn
Copy link
Contributor

@vogtn vogtn commented Aug 19, 2019

…nally only people picker has function comments

  • 0️⃣ ❗ Ensure all code is documented through tsdocs

PR Type

Refactoring (no functional changes, no api changes)
Documentation content changes

Description of the changes

Adding tsdocs comments to all component properties.

Referencing #115:

  • 0️⃣ ❗ Ensure all code is documented through tsdocs

PR checklist

  • Added tests and all passed
  • All public classes and methods have been documented
  • Added appropriate documentation
  • License header has been added to all new source files
  • Contains NO breaking changes

Other information

@vogtn vogtn requested a review from nmetulev August 20, 2019 20:55
Copy link
Contributor

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

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

Couple notes:

  1. Now that we've merged the tslint pr, it should help define which properties, methods, classes, etc need documentation.
  2. I've noticed you are not documenting classes, enums, etc - make sure you do those as well - tslint should help here (things like private properties and methods are not required for example)
  3. I've notices you are not documenting all the providers and graph source files, need to make sure everything is documented. If that's the case, ensure you've made that clear in this PR so we ensure we track other source files that need tsdocs

src/components/mgt-agenda/mgt-agenda.ts Outdated Show resolved Hide resolved
src/components/mgt-agenda/mgt-agenda.ts Outdated Show resolved Hide resolved
@vogtn
Copy link
Contributor Author

vogtn commented Aug 23, 2019

@nmetulev please re-review, I'll merge after bug fixes for people-picker

@nmetulev nmetulev mentioned this pull request Aug 23, 2019
23 tasks
@nmetulev
Copy link
Contributor

nmetulev commented Aug 27, 2019

This PR is only documenting the components and that's ok as we are updating the other source files in other PRs. Created #133 to track the rest of the work. Once you have resolved this last one comment, we can merge this

@nmetulev nmetulev merged commit 64d193b into master Aug 28, 2019
@nmetulev nmetulev deleted the nivogt/tsdocs-update branch August 28, 2019 03:43
shweaver-MSFT pushed a commit that referenced this pull request Jun 8, 2020
#125)

* [all] updating components with tsdocs comments to properties, additionally only people picker has function comments

* [mgt-agenda] updating tsdocs

* [mgt-login] updating tsdocs

* [people] updating ts docs

* [people-picker] updating tsdocs

* [mgt-person] updating tsdocs

* [mgt-tasks] updating tsdocs

* [mgt-login] additional ts documentation
shweaver-MSFT pushed a commit that referenced this pull request Jun 10, 2020
#125)

* [all] updating components with tsdocs comments to properties, additionally only people picker has function comments

* [mgt-agenda] updating tsdocs

* [mgt-login] updating tsdocs

* [people] updating ts docs

* [people-picker] updating tsdocs

* [mgt-person] updating tsdocs

* [mgt-tasks] updating tsdocs

* [mgt-login] additional ts documentation
shweaver-MSFT pushed a commit that referenced this pull request Jun 10, 2020
#125)

* [all] updating components with tsdocs comments to properties, additionally only people picker has function comments

* [mgt-agenda] updating tsdocs

* [mgt-login] updating tsdocs

* [people] updating ts docs

* [people-picker] updating tsdocs

* [mgt-person] updating tsdocs

* [mgt-tasks] updating tsdocs

* [mgt-login] additional ts documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants