Skip to content
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

Create a flutter example application that uses multiple client implementations #1040

Merged
merged 32 commits into from
Nov 7, 2023

Conversation

brianquinlan
Copy link
Collaborator

@brianquinlan brianquinlan commented Nov 2, 2023

  • Creates an example flutter application (in the style of the flutter gallery) that demonstrates package:http including:
    • how to do configuration (including web configuration)
    • how to pass Client instances through the app
    • simple usage
  • Updates the cronet_http and cupertino_http examples to match the new application, except where this would increase the complexity too much.
  • The application was created with flutter create -t app. Some interesting files:
    • README.md
    • pkgs/cronet_http/example/lib/main.dart
    • pkgs/cronet_http/example/lib/main.dart
    • pkgs/flutter_http_example/README.md
    • pkgs/flutter_http_example/lib/*
    • pkgs/flutter_http_example/mono_pkg.yaml
    • pkgs/flutter_http_example/pubspec.yaml
    • pkgs/flutter_http_example/test/widget_test.dart

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@github-actions github-actions bot added package:cupertino_http Issues related to package:cupertino_http package:cronet_http infra labels Nov 2, 2023
@brianquinlan brianquinlan changed the title Create a flutter example application Create a flutter example application that uses multiple client implementations Nov 2, 2023
Copy link
Member

@natebosch natebosch left a comment

Choose a reason for hiding this comment

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

I did not look at stuff other than the Dart. Consider getting a second review from someone with Flutter experience.

pkgs/cronet_http/example/lib/main.dart Outdated Show resolved Hide resolved
pkgs/cupertino_http/example/lib/main.dart Outdated Show resolved Hide resolved
pkgs/flutter_http_example/README.md Outdated Show resolved Hide resolved
@@ -0,0 +1,5 @@
# Launch Screen Assets

You can customize the launch screen with your own desired assets by replacing the image files in this directory.
Copy link
Member

Choose a reason for hiding this comment

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

Is it typical flutter practice to check in these generated files?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pkgs/flutter_http_example/lib/book.dart Show resolved Hide resolved
pkgs/flutter_http_example/lib/book.dart Outdated Show resolved Hide resolved
pkgs/flutter_http_example/lib/book.dart Outdated Show resolved Hide resolved
pkgs/flutter_http_example/lib/book.dart Outdated Show resolved Hide resolved
@@ -0,0 +1,155 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is copied from pkgs/cupertino_http so I think that this copyright is correct.

@brianquinlan brianquinlan merged commit b2717ba into dart-lang:master Nov 7, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:cronet_http package:cupertino_http Issues related to package:cupertino_http
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants