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

🛠 Tooling: Prevent Templates from defining --repository or --owner to values that can't be stringified into a repository locator #136

Closed
3 tasks done
JoshuaKGoldberg opened this issue Jan 15, 2025 · 3 comments · Fixed by #160
Assignees
Labels
area: documentation Improvements or additions to documentation ("docs") area: tooling Managing the repository's maintenance 🛠️ status: accepting prs Please, send a pull request to resolve this! 🙏
Milestone

Comments

@JoshuaKGoldberg
Copy link
Owner

JoshuaKGoldberg commented Jan 15, 2025

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Overview

A few places in code define owner and repository as options that must exist:

export type MinimumOptionsShape = AnyOptionalShape & RequiredOptionsShape;
export interface RequiredOptions {
owner: string;
repository: string;
}
export interface RequiredOptionsShape {
owner: z.ZodOptional<z.ZodString>;
repository: z.ZodOptional<z.ZodString>;
}

That's not enforced. Repositories can override those options, sort of, maybe. It's not clear what happens. Which is not great: we wouldn't want them to, say, override owner to a z.number().

I'd like to:

  • Have the tooling throw an error if a template tries to define either as an option
  • Pipe MinimumOptionsShape through places that might find it useful
  • Document this in https://www.create.bingo/cli#flags
  • Document this in the templating engine docs, in some way

Additional Info

It would be nice to represent this in TypeScript types, but I couldn't figure out a nice way with the type parameter constraints...

💝

@JoshuaKGoldberg JoshuaKGoldberg added area: documentation Improvements or additions to documentation ("docs") status: accepting prs Please, send a pull request to resolve this! 🙏 area: tooling Managing the repository's maintenance 🛠️ labels Jan 15, 2025
@JoshuaKGoldberg JoshuaKGoldberg added this to the Blocks Launch milestone Jan 15, 2025
@JoshuaKGoldberg JoshuaKGoldberg self-assigned this Jan 16, 2025
@boneskull
Copy link
Contributor

Hi, I'm Bob Dobbs at Amalgamated Feed & Hay. I want to provide a template for my team to use, but all packages we make should be owned by amalgamated_feed_and_hay, so I want to provide z.literal('amalgamated_feed_and_hay') instead. Please don't harsh my mellow

@JoshuaKGoldberg
Copy link
Owner Author

Haha great point. How about: they can be defined and overridden, but it must be some kind of string?

@JoshuaKGoldberg JoshuaKGoldberg changed the title 🛠 Tooling: Prevent Templates from defining --repository or --owner 🛠 Tooling: Prevent Templates from defining --repository or --owner to non-strings Jan 17, 2025
@boneskull
Copy link
Contributor

a string might be too strict

I guess my overall point is to freely allow all manner of nonsense input unless you are really, really sure it will fundamentally break functionality

@JoshuaKGoldberg JoshuaKGoldberg changed the title 🛠 Tooling: Prevent Templates from defining --repository or --owner to non-strings 🛠 Tooling: Prevent Templates from defining --repository or --owner to values that can't be stringified into a repository locator Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation Improvements or additions to documentation ("docs") area: tooling Managing the repository's maintenance 🛠️ status: accepting prs Please, send a pull request to resolve this! 🙏
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants