Skip to content

split show template#2954

Draft
goosys wants to merge 1 commit intothoughtbot:mainfrom
goosys:split-show-template-2
Draft

split show template#2954
goosys wants to merge 1 commit intothoughtbot:mainfrom
goosys:split-show-template-2

Conversation

@goosys
Copy link
Copy Markdown
Contributor

@goosys goosys commented Dec 4, 2025

@goosys
Copy link
Copy Markdown
Contributor Author

goosys commented Dec 5, 2025

Memo (just thinking out loud)

  • local_assigns feels too loose — maybe we should pass only the required locals?
  • The partial file names probably need another look.
    • We'll be doing similar work in form/index, so it might be better to align the naming.

@nickcharlton
Copy link
Copy Markdown
Member

Now some time has passed, do you have any more toughts on the local_assigns and partials naming?

@goosys goosys marked this pull request as draft March 26, 2026 04:54
@goosys
Copy link
Copy Markdown
Contributor Author

goosys commented Mar 26, 2026

It’s been a while, and I’m a bit fuzzy on what the original concern was, but looking at it again, both options seem fine to me.

If there are no concerns about the approach of splitting show, I can go ahead and split index and form in the same way and put together a draft PR. Then we can review those and make a final decision based on that -- what do you think?

@goosys goosys force-pushed the split-show-template-2 branch from 4c3769d to 6c79ced Compare March 26, 2026 09:48
@nickcharlton
Copy link
Copy Markdown
Member

Yeah, sounds like a good plan.

Re: local_assigns, I think we just want to ensure we don't inadvertently leak lots of context into a page, but we also don't want to make people's job overriding stuff harder unnecessarily.

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.

2 participants