Skip to content

Conversation

@jdolle
Copy link
Collaborator

@jdolle jdolle commented Jun 3, 2025

Background

https://linear.app/the-guild/issue/CONSOLE-1245/collaborative-schema-designerproposer

Our goal is to build a space where developers/teams can come together and collaborate on schema. "Schema Proposals", as it's currently named, is a feature that would do just this. By allowing users to submit a proposed change and others to review and comment.

Description

This is not the full feature, but it lays some of the groundwork for this feature.
I have a proposed database structure, initial graphql API, and am gradually adding additional elements to this feature branch.

@jdolle jdolle self-assigned this Jun 3, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jun 3, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch schema-proposals-schema

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2025

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@graphql-hive/cli 0.57.0-alpha-20251210002603-00cddfa4df7c30b57b388c817428598d24c44c66 npm ↗︎ unpkg ↗︎
hive 8.13.0-alpha-20251210002603-00cddfa4df7c30b57b388c817428598d24c44c66 npm ↗︎ unpkg ↗︎

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2025

📚 Storybook Deployment

The latest changes are available as preview in: https://pr-6836.hive-storybook.pages.dev

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2025

💻 Website Preview

The latest changes are available as preview in: https://pr-6836.hive-landing-page.pages.dev

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2025

🐋 This PR was built and pushed to the following Docker images:

Targets: build

Platforms: linux/amd64

Image Tag: 00cddfa4df7c30b57b388c817428598d24c44c66

@jasonkuhrt
Copy link
Contributor

Will look tomorrow!

Copy link
Contributor

@jasonkuhrt jasonkuhrt left a comment

Choose a reason for hiding this comment

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

Nice! Left a few odd thoughts on the data model fwiw 🙌

Comment on lines 150 to 158
CREATE TABLE IF NOT EXISTS "schema_proposal_comments"
(
id UUID PRIMARY KEY DEFAULT uuid_generate_v4 ()
, user_id UUID REFERENCES users (id) ON DELETE SET NULL
, body TEXT NOT NULL
, created_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
, updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
, schema_proposal_review_id UUID REFERENCES schema_proposal_reviews (id) ON DELETE CASCADE
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we bake in more semantics here rather than just free form? For example maybe a category enum.

type: kudos | confused | disagree | idea

The thinking is that structured semantics here would allow some interesting UX downstream, such as us being able to present the user with triaged comments e.g. "blocking" | "optional" etc. to reduce review process overhead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is a great idea but it might belong on the review object and is better suited to a v2.

For more context -- One constraint of the current table design is that every comment must be tied to a review. This is because I opted to use sql and I wanted to avoid complicated merging + pagination logic in the UI to display multiple sets or chronological data at the same time.
If using nosql then this could all be in the same table and have different structures.

@jdolle jdolle changed the title WIP: schema proposals Schema proposals beginnings Dec 8, 2025
@jdolle jdolle marked this pull request as ready for review December 8, 2025 20:16
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was in the midst of factoring it out for a followup PR, but based on our discussions we want to move forward as is and adjust.
I'll add the import back

Comment on lines 360 to 370
function defaultSchemaProposalIdentity(
args: { schemaProposalId: string | null } & Parameters<typeof defaultTargetIdentity>[0],
) {
const ids = defaultTargetIdentity(args);

if (args.schemaProposalId !== null) {
ids.push(`target/${args.targetId}/schemaProposal/${args.schemaProposalId}`);
}

return ids;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we introduce a new permission level here? This implies that we allow users to modify and view individual schema proposals within a target. 🤔

E.g. he that someone can see one specific schema proposal, isn't this permission on the target good enough for now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

💯
I was following what app deployments did without thinking. I've adjusted this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants