Skip to content

Add gen-sig script#193

Closed
aaronmallen wants to merge 1 commit into
dry-rb:mainfrom
aaronmallen:aa/signatures
Closed

Add gen-sig script#193
aaronmallen wants to merge 1 commit into
dry-rb:mainfrom
aaronmallen:aa/signatures

Conversation

@aaronmallen
Copy link
Copy Markdown
Member

This is adds the gen-sig script to generate signatures as well as a signatures for the project.

Copy link
Copy Markdown
Contributor

@alassek alassek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very promising! ✨

I've wanted to marry fxn's inline annotation convention with rbs sigs for a long time, but never made much progress on it. Looks like Soutaro had a similar idea.

My code review comments are mostly just nitpicks. As for how to integrate this, probably a precommit hook. We could potentially do something like repo-sync, which only makes commits if its templates generate changes, but that would take some effort.

Comment thread bin/gen-sig Outdated
Comment thread bin/gen-sig Outdated
Comment thread bin/gen-sig Outdated
Comment thread bin/gen-sig Outdated
Comment thread bin/gen-sig Outdated
@aaronmallen aaronmallen force-pushed the aa/signatures branch 3 times, most recently from 307758b to 6f267a4 Compare October 15, 2025 22:15
This is adds the `gen-sig` script to generate signatures as well as a
signatures for the project.
@aaronmallen
Copy link
Copy Markdown
Member Author

@alassek how do we feel about reducing the scope of this work a bit... I'm running into some walls where things aren't documented and I'm having a hard time groking their shapes. What if we reduce the scope of this PR to incorporating the script and follow up with more refined signatures in future PRs?

@aaronmallen
Copy link
Copy Markdown
Member Author

closing in favor of #194

@alassek
Copy link
Copy Markdown
Contributor

alassek commented Oct 24, 2025

@aaronmallen by no means would I expect you to annotate the whole thing. Just the documented public API would be sufficient.

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