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

Add Guides 2.0 #90

Closed
wants to merge 27 commits into from
Closed

Add Guides 2.0 #90

wants to merge 27 commits into from

Conversation

swilgosz
Copy link
Member

@swilgosz swilgosz commented Jun 9, 2021

Overview

Guides for Hanami 2.0

  • Getting Started
  • Architecture
    • Overview
  • Router
    • Overview
    • Basic Usage
    • Advanced Usage
    • Testing
  • Actions
  • Views
  • Persistance
  • Validations
  • Slices
  • Assets
  • Utils
  • Helpers
  • Mailers
  • Command Line
  • Upgrade Notes

@swilgosz swilgosz requested a review from jodosha June 9, 2021 12:48
@swilgosz swilgosz self-assigned this Jun 9, 2021
Copy link
Member

@cllns cllns left a comment

Choose a reason for hiding this comment

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

🎉 Excited we're getting started with these 2.0 guides. Great work :)

content/v2.0/router/overview.md Show resolved Hide resolved
content/v2.0/router/overview.md Outdated Show resolved Hide resolved
content/v2.0/router/overview.md Outdated Show resolved Hide resolved
content/v2.0/router/overview.md Show resolved Hide resolved
content/v2.0/router/overview.md Show resolved Hide resolved
content/v2.0/router/overview.md Outdated Show resolved Hide resolved
content/v2.0/router/overview.md Outdated Show resolved Hide resolved
# /config/routes.rb

Hanami.application.routes do
slice :main, at: "/" do
Copy link
Member

Choose a reason for hiding this comment

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

Should we explain how slice routing works here too, at least briefly? There will be more guides about slices, of course, but this will be a new concept to almost everyone so briefly mentioning it's something new to learn could be useful (though the routing portion here is almost trivial and obvious).

Copy link
Member Author

Choose a reason for hiding this comment

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

This is also a topic I briefly mentioned in the basic_usage - Let's discuss if it's better to put it here when I'll push remaining docs :). I've not a strong opinion about it yet.

Add note about Hanami v1.3 requiring Ruby 2

## Mounting an application

In Hanami, you can mount any rack application in desired route. If we want to do it, you should use the `mount` keyword.
Copy link
Member

Choose a reason for hiding this comment

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

This is in the rack configuration, right? config.ru rather than a route files?

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, is it? I will check, thanks! I know it was in the config in 1.3, but I think I saw it changed in new tests.

Will get back to it! cc: @jodosha

Copy link
Member

Choose a reason for hiding this comment

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

Nope, it's the router.

@swilgosz I strongly advice to wrap those lines into the following block:

# config/routes.rb
module MyApp
  class ApplicationRoutes < Hanami::Application::Routes
    mount # ...
  end
end

Please be aware that this syntax may change, but at least it's crystal clear of what we're talking about: application routes, and not config.ru.


I would not use "keyword" but "method" instead. Usually keywords are langs constructs.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jodosha ATM Hanami2APplication template does not inherit from Hanami::Application::Routes, but rather passes a block to the main class

https://github.com/hanami/hanami-2-application-template/blob/main/config/routes.rb

content/v2.0/router/advanced-usage.md Outdated Show resolved Hide resolved

It generates:

<table class="table table-bordered table-striped">
Copy link
Member

Choose a reason for hiding this comment

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

Can we change this to (GitHub-flavored) Markdown tables? https://github.github.com/gfm/#tables-extension-

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. I copied it from existing docs, but if other syntax works better, I don't mind.

content/v2.0/router/advanced-usage.md Outdated Show resolved Hide resolved
content/v2.0/router/advanced-usage.md Outdated Show resolved Hide resolved
content/v2.0/router/advanced-usage.md Outdated Show resolved Hide resolved
content/v2.0/router/basic-usage.md Show resolved Hide resolved
content/v2.0/router/basic-usage.md Outdated Show resolved Hide resolved
content/v2.0/router/basic-usage.md Outdated Show resolved Hide resolved
content/v2.0/router/basic-usage.md Outdated Show resolved Hide resolved
content/v2.0/router/overview.md Outdated Show resolved Hide resolved
jodosha and others added 18 commits June 30, 2021 00:50
Reason: We do not recommend handling requests this way, but it is worth mentioning
How it works and that it is possible.
lib
├── bookshelf
│ ├── action.rb
│ ├── entities
Copy link

Choose a reason for hiding this comment

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

@swilgosz No biggie, but maybe a tailing slash for directories would improve the readability, e.g.

├── entities/
├── entities.rb
├── foodir/
    └── barfile

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea! BTW: not sure, if you are aware, but you can suggest changes inside reviews, that I can accept directly as your commit

Copy link

Choose a reason for hiding this comment

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

@swilgosz IC, will do next time.

@swilgosz
Copy link
Member Author

Closed in favour of #97 #98 #99 and other per-chapter prs

@swilgosz swilgosz closed this Dec 21, 2021
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.

5 participants