Open
Conversation
Owner
Author
|
Hi @OughtPuts 👋 nice to meet you. I've added you to this PR as a reviewer! |
OughtPuts
reviewed
Jun 30, 2024
Collaborator
OughtPuts
left a comment
OughtPuts
left a comment
There was a problem hiding this comment.
I thought this was really good and really clear, particularly for those picking it up for the first time! Saw a couple of opportunities for extra clarification as I went through which I commented, but I wouldn't say any of them are major blockers, just ideas. Enjoyed reading - thank you!
Co-authored-by: Matheus Richard <matheusrichardt@gmail.com> Co-authored-by: Bartosz Łęcki <bart.lecki@gmail.com> Co-authored-by: Lucas Fernandes <lsfernandes92@gmail.com>
Co-authored-by: Matheus Richard <matheusrichardt@gmail.com> Co-authored-by: Lucas Fernandes <lsfernandes92@gmail.com>
…o bhumi-guides-routing
Co-authored-by: Matheus Richard <matheusrichardt@gmail.com>
OughtPuts
reviewed
Aug 22, 2024
Collaborator
OughtPuts
left a comment
OughtPuts
left a comment
There was a problem hiding this comment.
Nice job @bhumi1102! Just a few tiny typos I spotted on final read through that we may want to clear up.
| The request is matched to the `UsersController` class's `show` method with `{ id: '17' }` in the `params` hash. | ||
|
|
||
| The `to:` option expects a `controller#action` format when passed a string. Alternatively, You can pass a symbol and use the `action:` option, instead of `to:`. You can also pass a string without a `#`, in which case the `controller:` option is used instead to `to:`. For example: | ||
|
|
Collaborator
There was a problem hiding this comment.
Argh sorry, found a typo. Should this be 'in which case the controller: option is used instead OF to:.
guides/source/routing.md
Outdated
| then the router will generate the path `/patients/17`. This reduces the brittleness of your view and makes your code easier to understand. Note that the id does not need to be specified in the route helper. | ||
| The router will generate the path `/users/17` from `user_path(@user)`. Using the `user_path` helper allows you to avoid having to hard-code a path in your views. This is helpful if you eventually move the route to a different URL, as you won't need to update the corresponding views. | ||
|
|
||
| It also generates `user_url`, which has a similar purpose. While `user_path` generates a relative url like `/users/17`, `user_url` generates an absolute url something like `https://example.com/users/17` in the above example. |
Collaborator
There was a problem hiding this comment.
the 'something like' reads a bit funny here. Consider rewording to "... generates an absolute url. For the above example this would be https://example.com/users/17."
guides/source/routing.md
Outdated
|
|
||
| Each of these `_path` helpers also have a corresponding `_url` helper (such as `photos_url`) which returns the same path prefixed with the current host, port, and path prefix. | ||
|
|
||
| TIP: The prefix used before "_path" and "_url" is the route name and can be identified by looking at the "prefix" column of the `rails routes` command output. To learn more see [Listing existing routes](#listing-existing-routes) below. |
Collaborator
There was a problem hiding this comment.
'Listing Existing Routes' should be capitalized.
| TIP: If your application has many RESTful routes, using `:only` and `:except` to | ||
| generate only the routes that you actually need can cut down on memory use and | ||
| speed up the routing process by eliminating [unused | ||
| routed](#listing-unused-routes). |
Collaborator
There was a problem hiding this comment.
Link text should be 'unused routes'.
Co-authored-by: Petrik de Heus <petrik@deheus.net> Co-authored-by: Matheus Richard <matheusrichardt@gmail.com>
…o bhumi-guides-routing
bhumi1102
pushed a commit
that referenced
this pull request
Dec 11, 2025
* Active storage immediate variants (#6) * This commit introduces variant generation strategies for Active Storage, providing more flexible control over when variants are created. - **:lazily** - variants created dynamically when requested (default) - **:later** - variants created in background after attachment - **:immediately** - variants created simultaneously with attachment ```ruby has_one_attached :avatar_with_lazy_variants do |attachable| attachable.variant :lazy_thumb, resize_to_limit: [3, 3], process: :lazy attachable.variant :default_thumb, resize_to_limit: [4, 4] end has_one_attached :avatar_with_later_variants do |attachable| attachable.variant :later_thumb, resize_to_limit: [2, 2], process: :later end has_one_attached :avatar_with_immediate_variants do |attachable| attachable.variant :immediate_thumb, resize_to_limit: [1, 1], process: :immediately end ``` * Active Storage: make Variant#processed? and VariantWithRecord#processed? public Apps can now check whether variants have been generated. Useful for precise control of variant usage or generation. Co-authored-by: Tom Rossi <tomrossi7@users.noreply.github.com> * Deprecate `preprocessed: true` in favor of `process: :later` For removal in Rails 9.0. Co-authored-by: Tom Rossi <tomrossi7@users.noreply.github.com> * Document immediate variants: guide and changelog Co-authored-by: Tom Rossi <tomrossi7@users.noreply.github.com> * Making ActiveStorage::Preview#processed? a public method Removing space * Use consistent naming for transformations parameter (#7) ActiveStorage uses the plural `transformations` for a single hash of transformation options (e.g., `{ resize_to_limit: [100, 100] }`). To differentiate between a single transformation hash and an array of transformation hashes, this commit introduces `transformations_array` for arrays of transformations, while retaining transformations for individual hashes. * Fixing changelog * Rename transformations_array to variants Aligns with `CreateVariantsJob` naming and `ActiveStorage` terminology where variants represent the result of transformations applied to the original blob. Fixing create_preview_image --------- Co-authored-by: Jeremy Daer <jeremy@rubyonrails.org> Co-authored-by: Tom Rossi <tomrossi7@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi all - this guide is ready for internal review.
Note: seeing intermittent build failure due to "error creating build container" from docker. I'll look into it but it doesn't have anything to do with the guides. So safe to review thisHere are the notes from the TODO:
--> Good point. Tried to move the 3 or 4 sub-sections related to constraints, but couldn't decide where they fit, after non-resource routes or before, and how to transition into them...leaving it as is for now.
--unusedoption to rails routes. (Reference)