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

🚀 Feature: Switch request Creations to a serializable and testable format #65

Open
2 tasks done
JoshuaKGoldberg opened this issue Dec 17, 2024 · 0 comments
Open
2 tasks done
Assignees
Labels
status: accepting prs Please, send a pull request to resolve this! 🙏 type: feature New enhancement or request
Milestone

Comments

@JoshuaKGoldberg
Copy link
Owner

JoshuaKGoldberg commented Dec 17, 2024

Bug Report Checklist

Overview

I'd noted in #64:

Note that this is the first Creation form that isn't directly serializable. Files and scripts can all be made JSON; requests now introduce functions. I'd been hoping to keep everything serializable - but being able to instead get nice type completion with using Octokit APIs is just more useful.

If someone wants serializable requests in the future, I suppose they could file a feature request asking for the JSON-like equivalent of this. Something like:

{
  "fetcher": "octokit",
  "method": ["rest", "repos", "update"],
  "send": {
    allow_auto_merge: true,
    // ...
  }
}

...where fancy TypeScript types determine the send data to be Parameters<typeof octokit.rest.repos.update>[0]. That's a lot more work. Perhaps a future improvement.

I also don't love that explicit id in there. Previous Creations are auto-mergable without them; this one is manual. I suppose that's also tied in with it not being serializable. Perhaps a future improvement.

Another downside of the non-serializable requests format is the not-very-informative [Function] showing up in creation snapshots. For example, in working on JoshuaKGoldberg/create-typescript-app#1775, I'm seeing the test output for a Block that creates requests as:

const creation = testBlock(blockRepositoryBranchProtection, {
	addons: {
		requiredStatusChecks: ["build", "test"],
	},
	options: optionsBase,
});

expect(creation).toMatchInlineSnapshot(`
	{
	  "requests": [
	    {
	      "id": "branch-protection",
	      "send": [Function],
	    },
	  ],
	}
`);

As a followup, I'd like to eventually pursue the avenue of a fully serializable, type-safe requests format.

Additional Info

Marking as status: blocked for the time being. It doesn't block full CTA parity. But I will want it to block a full launch.

💖

@JoshuaKGoldberg JoshuaKGoldberg added type: feature New enhancement or request status: blocked Waiting for something else to be resolved labels Dec 17, 2024
@JoshuaKGoldberg JoshuaKGoldberg added this to the Ready to Launch milestone Dec 17, 2024
@JoshuaKGoldberg JoshuaKGoldberg self-assigned this Dec 17, 2024
@JoshuaKGoldberg JoshuaKGoldberg added status: accepting prs Please, send a pull request to resolve this! 🙏 and removed status: blocked Waiting for something else to be resolved labels Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Please, send a pull request to resolve this! 🙏 type: feature New enhancement or request
Projects
None yet
Development

No branches or pull requests

1 participant