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

Update docs to recommend upfront module definition (i.e. Types = Dry.Types()) #432

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

timriley
Copy link
Member

@timriley timriley commented Jan 4, 2022

As agreed, this is our recommendation to resolve the issue raised in #422.

Most of our code examples depend on a ::Types module having been already created, so to make sure those examples are as clear as possible, I've also added Types = Dry.Types() in front of any grouping of code examples.

Made a few other very minor adjustments along the way, too.

Resolves #422

@timriley timriley requested a review from solnic as a code owner January 4, 2022 00:44
@timriley
Copy link
Member Author

timriley commented Jan 4, 2022

@solnic @flash-gordon — are you happy with this? If so, I'll merge this and actually make sure we have a 1.5 version of the docs published at dry-rb.org.

@@ -15,7 +15,7 @@ Built-in types are grouped under 6 categories:

### Categories

Assuming you included `Dry::Types` ([see instructions](docs::getting-started)) in a module called `Types`:
Assuming you've defined your own `Types` module ([see instructions](docs::getting-started)):
Copy link

Choose a reason for hiding this comment

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

How do you feel about changing the assertion to one that would be more indicative that they should do this, rather than conditional that they might have.

Suggested change
Assuming you've defined your own `Types` module ([see instructions](docs::getting-started)):
After you've defined your own `Types` module ([see instructions](docs::getting-started)):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Params namespace disapearing
5 participants