Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: add dynamic parameters websocket endpoint#17165

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
aslilac merged 10 commits intomainfromlilac/dynamic-parameters-endpoint
Apr 10, 2025

Conversation

aslilac
Copy link
Member

@aslilacaslilac commentedMar 28, 2025•
edited
Loading

Adds a new /dynamic-parameters websocket endpoint, some tests for it, and a small bit of generic machinery.

Don't be scared by the line count! This PR is only like 400 lines of actual code, the vast majority of the diff is from go.sum

@aslilacaslilac changed the base branch frommain tolilac/files-cacheApril 1, 2025 21:55
@aslilacaslilacforce-pushed thelilac/dynamic-parameters-endpoint branch from3af6a57 to18ff0b4CompareApril 1, 2025 21:58
Base automatically changed fromlilac/files-cache tomainApril 2, 2025 22:42
@aslilacaslilac requested a review fromEmyrkApril 2, 2025 23:39
@aslilacaslilac marked this pull request as ready for reviewApril 3, 2025 23:50
@aslilacaslilacforce-pushed thelilac/dynamic-parameters-endpoint branch from00de0bd toa54058eCompareApril 7, 2025 22:32
Copy link
Member

@EmyrkEmyrk left a comment

Choose a reason for hiding this comment

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

This PR is really clean 👍

I left some notes. It would be ideal if we could extract most of the logic from the endpoint into it's own little pkg incoder. Then we could create some nice tabled tests a lot easier imo.

Comment on lines -91 to +94
"tailscale.com/types/opt.Bool": config.OverrideNullable(config.OverrideLiteral(bindings.KeywordBoolean)),
"tailscale.com/types/opt.Bool": config.OverrideNullable(config.OverrideLiteral(bindings.KeywordBoolean)),
"github.com/hashicorp/hcl/v2.Expression": config.OverrideLiteral(bindings.KeywordUnknown),
Copy link
Member

Choose a reason for hiding this comment

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

I was leaving a few comments aboutguts, and honestly the syntax for overriding types is just... unfortunate right now.

I ran out of time when working on guts, and threw in type overrides in the current state just to make things possible, but it's not easy to use.

So I just made a PR that makes the changes I was proposing:#17339

One day I'll fixup guts and make this easier to use.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

is it alright if we merge your pr into main after merging this one?

Emyrk reacted with thumbs up emoji
Copy link
Member

Choose a reason for hiding this comment

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

That is fine

@aslilacaslilac requested a review fromEmyrkApril 10, 2025 19:39
Comment on lines -91 to +94
"tailscale.com/types/opt.Bool": config.OverrideNullable(config.OverrideLiteral(bindings.KeywordBoolean)),
"tailscale.com/types/opt.Bool": config.OverrideNullable(config.OverrideLiteral(bindings.KeywordBoolean)),
"github.com/hashicorp/hcl/v2.Expression": config.OverrideLiteral(bindings.KeywordUnknown),
Copy link
Member

Choose a reason for hiding this comment

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

That is fine

@aslilacaslilac merged commit859dd2f intomainApr 10, 2025
34 checks passed
@aslilacaslilac deleted the lilac/dynamic-parameters-endpoint branchApril 10, 2025 20:08
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsApr 10, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@EmyrkEmyrkEmyrk approved these changes

Assignees

@aslilacaslilac

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@aslilac@Emyrk
[8]ページ先頭
©2009-2025 Movatter.jp