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

docs: Define Container, fx.Provide, fx.Supply, fx.Invoke #1156

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

abhinav
Copy link
Collaborator

@abhinav abhinav commented Feb 9, 2024

Adds documentation defining the concept of a container
and a couple of the core intrinsic operations that a container suppotrs.

In the future, this should include fx.Decorate as an operation,
and can link to more thorough documentation about decoration separately.

Adds documentation defining the concept of a container
and a couple of the core intrinsic operations that a container suppotrs.

In the future, this should include `fx.Decorate` as an operation,
and can link to more thorough documentation about decoration separately.
@abhinav
Copy link
Collaborator Author

abhinav commented Feb 9, 2024

Screenshots:
image

image

Copy link

codecov bot commented Feb 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (89f4a90) 98.74% compared to head (3b057eb) 98.74%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1156   +/-   ##
=======================================
  Coverage   98.74%   98.74%           
=======================================
  Files          30       30           
  Lines        2950     2950           
=======================================
  Hits         2913     2913           
  Misses         30       30           
  Partials        7        7           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@abhinav
Copy link
Collaborator Author

abhinav commented Feb 12, 2024

error: container.md:57: link #when-to-use-fx-supply, normalized to: link /home/runner/work/fx/fx/docs/container.md#when-to-use-fx-supply, existing ids: [container providing-values when-to-use-fxsupply using-values when-to-use-fxinvoke]: file exists, but does not have such id

Hm, mdox and vuepress disagree here. I'll just use an explicit anchor.

@abhinav
Copy link
Collaborator Author

abhinav commented Feb 12, 2024

Never mind, it appears that mdox is just broken for this case.
It doesn't recognize explicit anchors added with <a id="foo"></a>, and it incorrectly drops the '.' when guessing the id.

I'm going to work around this for now, but we should consider dropping its link validation completely in that case.

Copy link
Contributor

@JacobOaks JacobOaks left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for this addition. LGTM - just left one question.

Comment on lines +13 to +23
```go
package fx

type App
func New(opts ...Option) *App
func (app *App) Run()

type Option
func Provide(constructors ...interface{}) Option
func Invoke(funcs ...interface{}) Option
```
Copy link
Contributor

@JacobOaks JacobOaks Feb 12, 2024

Choose a reason for hiding this comment

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

I think your intention is to show some relevant functions for this section and their relevent types, but this throws me off a little because its formatted as a code block and yet isn't really valid Go syntax. I think it's fine, just something to be aware of. Is this a common format for trying to portray this kind of information that I'm not aware of?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This formatting was borrowed from how godoc shows things:

image

I'm not tied to it, though. We can change it if you'd like.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay! Makes sense. I think this is good then.

@JacobOaks JacobOaks merged commit 4c7a22b into uber-go:master Feb 12, 2024
12 checks passed
@abhinav abhinav deleted the abg/concepts-container branch February 12, 2024 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants