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

[2.2] Clean up Web app guide #244

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

Conversation

cllns
Copy link
Member

@cllns cllns commented Jul 16, 2024

Some of these are simple fixes for mistakes, but others are my preferences for explaining what's happening. I know we don't want to overload the reader, but I think some info can be helpful. Feel free to push back, or only take some of thse.

I also updated the screenshot, but edited the DOM so hopefully we can use that same image for all our 2.2 releases, instead of having to make a new one with each patch release to keep it accurate.

I'll also go through the API one, but I want to get feedback on this one first.

@cllns cllns requested a review from timriley July 16, 2024 21:27
@cllns
Copy link
Member Author

cllns commented Jul 17, 2024

Some other notes I took:

  • Should we change the book titles for switching from the fixtures to the DB records? Right now it's just the , by AUTHOR that's different, which could be hard to pick up on, since the titles are there in both, and the same.

  • What do you think about renaming the feature specs to be what feature they're implementing for the user/app rather than their implementation? This might be too tied to my personal workflow but I find having distinct names helps when fuzzy searching files. Right now fuzzy-finding on books/index shows 6 files.
    Here's what I propose:

    spec/features/books/create_spec.rb           -> spec/features/add_book_spec.rb
    spec/features/books/index_spec.rb            -> spec/features/list_books_spec.rb
    spec/features/books/index/pagination_spec.rb -> spec/features/paginate_books_spec.rb
    spec/features/books/show_spec.rb             -> spec/features/show_book_spec.rb
    
  • Instead of using #one to get the book record, then introduce #one! right after that, what if we just jump to using #one! and mention that there's #one also. By default #one! will give them a better error. Especially if we take my propose changes about explaining materializing relations, that'd be a good place to talk about the difference.

Happy to implement any/all of these, just want feedback first.

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.

1 participant