Skip to content

update grpc & models to latest#149

Open
vishna wants to merge 1 commit intocachapa:masterfrom
vishna:master
Open

update grpc & models to latest#149
vishna wants to merge 1 commit intocachapa:masterfrom
vishna:master

Conversation

@vishna
Copy link

@vishna vishna commented Feb 4, 2026

TL;DR: I've updated something that updated something that needed me to update this. Now it works for me. Maybe it can be merged and published as new version.

I can improve my PR if it there's any attention from the repo owner - no need to rush tho, we all have our priorities. Great package btw! I think the regen.sh script could be better, e.g. lock to version of the repo and also check it out on the fly in an ignored folder.

@cachapa
Copy link
Owner

cachapa commented Feb 5, 2026

Thanks!

The only changes I'm unsure of were the ones to the readme. I'll annotate them.

Add Firedart to your `pubspec.yaml` file:

``` yaml
```yaml
Copy link
Owner

Choose a reason for hiding this comment

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

Why change the format here? Was it automated?

Copy link
Author

Choose a reason for hiding this comment

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

ah, yes sorry about that - I think some MD auto formatter in VSCode, my bad

Copy link
Owner

Choose a reason for hiding this comment

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

I figured, no worries don't waste time trying to fix this.


To regenerate them, you will need to check out both [googleapis](https://github.com/googleapis/googleapis) and [protobuf](https://github.com/google/protobuf).

**SIDENOTE**: `protobuf` doesn't seem to be needed anymore, you can just use those via `import 'package:protobuf/well_known_types/google/protobuf/wrappers.pb.dart';`
Copy link
Owner

Choose a reason for hiding this comment

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

If the protobuf dependency is no longer necessary can we remove it from pubspec.yaml?

In that case I would also remove this line from here since it doesn't impact library users, and add a note to the release notes.

Copy link
Author

Choose a reason for hiding this comment

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

You don't need to check out protobuf repo anymore and generate dart classes from it. These classes seem to come now as a part of protobuf package under well_known_types. That said - you still need protobuf package in pubspec but it's one less repo to check out now.

Copy link
Owner

Choose a reason for hiding this comment

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

That makes sense. In any case I think we can leave this note out since it's not relevant for users of this library.

@cachapa
Copy link
Owner

cachapa commented Feb 5, 2026

I think the regen.sh script could be better, e.g. lock to version of the repo and also check it out on the fly in an ignored folder.

I'm happy to accept those changes as well if you're willing to commit them.
Might make sense as a separate PR.

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