-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add connect-dart plugin #1638
Add connect-dart plugin #1638
Conversation
Signed-off-by: Sri Krishna Paritala <[email protected]>
Signed-off-by: Sri Krishna Paritala <[email protected]>
FROM dart:3.5.4-sdk AS build | ||
|
||
WORKDIR /build | ||
RUN curl -fsSL https://github.com/connectrpc/connect-dart/archive/refs/tags/v0.1.0.tar.gz --compressed -o connect-dart.tar.gz \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not downloading from pub.dev like the base types plugin because dart is unable to resolve dependencies because of the relative dev dependency. This is a fairly common practice.
plugins/connectrpc/dart/source.yaml
Outdated
@@ -0,0 +1,3 @@ | |||
source: | |||
dart_flutter: | |||
name: connectrpc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double-checking, I presume we will be publishing to pub.dev?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other words, is this the correct place to look for new releases instead of just github?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in a166d0d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was an issue with pub.dev
see #1638 (comment)
@@ -0,0 +1,16 @@ | |||
# syntax=docker/dockerfile:1.4 | |||
FROM dart:3.5.4-sdk AS build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the best way to know which dart version we should support?
Asking because there's an upstream issue where the directive is still at 2.x
Part of the reason we haven't updated this yet, e.g.,
https://github.com/bufbuild/plugins/blob/main/plugins/protocolbuffers/dart/v21.1.2/Dockerfile#L2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pubspec.yaml
files have the dart sdk range specified, as long as it satisfies that we should be able to upgrade
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay great, then we should be able to update the latest base types to the latest dart 3 version. That way we can keep both dart plugins on the same version.
Signed-off-by: Sri Krishna Paritala <[email protected]>
Co-authored-by: Michael Fridman <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. Just one nit on using git clone instead of curl the .zip for consistency
Signed-off-by: Sri Krishna Paritala <[email protected]>
Add connect-dart plugin