Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[camera] Introduce camera_web package #4151

Merged
merged 20 commits into from
Jul 16, 2021
Merged

[camera] Introduce camera_web package #4151

merged 20 commits into from
Jul 16, 2021

Conversation

bselwe
Copy link
Contributor

@bselwe bselwe commented Jul 12, 2021

Adds camera_web plugin, serves as a starting point for the Web platform implementation of camera.

  • Create the camera_web package
    • Copy the LICENSE file from the camera package
    • Add a CHANGELOG.md file
    • Define a pubspec.yaml file
    • Create a README.md file
  • Create CameraPlugin, the web implementation of the camera platform interface that throws unimplemented error for each method

Part of flutter/flutter#45297

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

@bselwe
Copy link
Contributor Author

bselwe commented Jul 12, 2021

@felangel @ditman

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM! Let's get this landed. I've only noted a few changes wrt the 0.0.1 version being too low, and some other nitpicks.

I've checked and the integration tests are already running successfully in the CI of this change, here:

https://cirrus-ci.com/task/6318685427073024?logs=drive#L20

Good stuff!

packages/camera/camera_web/CHANGELOG.md Outdated Show resolved Hide resolved
packages/camera/camera_web/README.md Outdated Show resolved Hide resolved
packages/camera/camera_web/example/README.md Outdated Show resolved Hide resolved
packages/camera/camera_web/example/build.yaml Outdated Show resolved Hide resolved
packages/camera/camera_web/pubspec.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@felangel felangel left a comment

Choose a reason for hiding this comment

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

Left a bunch of minor comments/nits but overall LGTM 🎉

@bselwe bselwe requested a review from ditman July 13, 2021 14:01
@stuartmorgan
Copy link
Contributor

Per the output:

The license block for these files is missing or incorrect:
/tmp/cirrus-ci-build/packages/camera/camera_web/lib/camera_web.dart

And indeed, that file has no license.

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM, my only concern is with publishing to pub a version of the plugin that doesn't do anything. I'll try to see if there's a way of merging this and having the auto-publish code ignore it until at least we have ported over all the functionality of the Photobooth (then we can call that 0.1.0 or whatever we want).

packages/camera/camera_web/example/pubspec.yaml Outdated Show resolved Hide resolved
@bselwe
Copy link
Contributor Author

bselwe commented Jul 15, 2021

Sounds good @ditman, thanks!

@stuartmorgan
Copy link
Contributor

LGTM, my only concern is with publishing to pub a version of the plugin that doesn't do anything. I'll try to see if there's a way of merging this and having the auto-publish code ignore it

You could mark it publish_to: none for now in the pubspec, with a comment explaining why.

@ditman
Copy link
Member

ditman commented Jul 16, 2021

This needs to be rebased with the latest master, I tried to do it myself but got a permission denied:

dit@dit:~/github/plugins$ git push
ERROR: Permission to VeryGoodOpenSource/plugins.git denied to ditman.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights and the repository exists.

@felangel
Copy link
Contributor

This needs to be rebased with the latest master, I tried to do it myself but got a permission denied:

dit@dit:~/github/plugins$ git push
ERROR: Permission to VeryGoodOpenSource/plugins.git denied to ditman.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights and the repository exists.

Just added you as a collaborator so you should have permission to push now 👍

@google-cla

This comment has been minimized.

@google-cla google-cla bot added cla: no and removed cla: yes labels Jul 16, 2021
@bselwe
Copy link
Contributor Author

bselwe commented Jul 16, 2021

It is rebased and awaits @felangel consent.

@felangel
Copy link
Contributor

@googlebot I consent.

@google-cla google-cla bot added cla: yes and removed cla: no labels Jul 16, 2021
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

Let's go!

@ditman ditman merged commit 57fb51f into flutter:master Jul 16, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 16, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 19, 2021
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
This first version is a no-op implementation of the platform_interface, but is the initial step to bring over the implementation from the Photobooth into flutter/plugins. Won't be published (for now).

Co-authored-by: Felix Angelov <[email protected]>
amantoux pushed a commit to amantoux/plugins that referenced this pull request Sep 27, 2021
This first version is a no-op implementation of the platform_interface, but is the initial step to bring over the implementation from the Photobooth into flutter/plugins. Won't be published (for now).

Co-authored-by: Felix Angelov <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants