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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(extension-health): add function call-order note #4410

Conversation

achrinza
Copy link
Member

Signed-off-by: Rifa Achrinza 25147899+achrinza@users.noreply.github.com

See also #4289

Adds a notice to inform users of the mandatory function calling order of this.component() and this.configure() to prevent a 404 HTTP error.

Checklist

馃憠 Read and sign the CLA (Contributor License Agreement) 馃憟

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

馃憠 Check out how to submit a PR 馃憟

@achrinza achrinza self-assigned this Jan 12, 2020
@achrinza achrinza added developer-experience Issues affecting ease of use and overall experience of LB users Extensions labels Jan 12, 2020
@achrinza achrinza force-pushed the @docs/health-readme-function-order-note branch 2 times, most recently from 06c8366 to 8e74834 Compare January 12, 2020 10:25
@achrinza achrinza changed the title docs(health): add function call-order note docs(extension-health): add function call-order note Jan 12, 2020
Copy link
Contributor

@derdeka derdeka left a comment

Choose a reason for hiding this comment

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

Note: this.configure() must be called before this.component() to prevent a
404 HTTP error. This is a
known issue.

I don't like the causality of '404 HTTP error'. Maybe just write:
"Note: this.configure() has to be called before this.component() to take effekt."

@achrinza achrinza force-pushed the @docs/health-readme-function-order-note branch from 8e74834 to ec015b3 Compare January 13, 2020 10:59
@achrinza
Copy link
Member Author

Note: this.configure() must be called before this.component() to prevent a
404 HTTP error. This is a
known issue.

I don't like the causality of '404 HTTP error'. Maybe just write:
"Note: this.configure() has to be called before this.component() to take effekt."

@derdeka Thanks for the suggestion, I've made the changes. Please let me know if there's any other issues.

@achrinza achrinza requested a review from derdeka January 13, 2020 11:02
@dougal83
Copy link
Contributor

Just wondering would a blue note be better to add emphasis?:

{% include note.html content="Lorem ipsum..." %}

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>

docs(extension-health): add function call-order note

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>

docs(extension-health): add function call-order note

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>

docs(extension-health): add function call-order note

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>
@achrinza achrinza force-pushed the @docs/health-readme-function-order-note branch from ec015b3 to 752b1de Compare January 13, 2020 11:57
@achrinza
Copy link
Member Author

Just wondering would a blue note be better to add emphasis?:

{% include note.html content="Lorem ipsum..." %}

@dougal83 I think that's a good idea; I've added the changes. LMK if there's any other changes we can add.

@raymondfeng raymondfeng merged commit 7c213af into loopbackio:master Jan 13, 2020
@achrinza achrinza deleted the @docs/health-readme-function-order-note branch January 20, 2020 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Issues affecting ease of use and overall experience of LB users Extensions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants