-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[cross_file] An abstraction to allow working with files across multiple platforms. #3260
Conversation
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.
Is there a way that we can preserve @jasonpanelli's authorship on this package? Can we maybe add him as a co-author of one of your commits, and then ensure that when we merge he stays as a co-author?
Thanks!
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@googlebot I consent. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@ditman thanks for the review. I have added the missing copyrights and immediately added @jasonpanelli as co-author. This however resulted in the CLA step failing since Jason now needs to also give his consent (which makes sense, but didn't think about it earlier). Also I did a |
Thanks for the extra effort @mvanbeusekom! @jasonpanelli, when you have time, can you please tell googlebot you consent to this change? Thanks! |
/cc @tugorez can you please also take a look at the new CrossFile, and see if it fits future usages or the ability to add extra metadata? Thanks!! |
Quick note @mvanbeusekom: there's already some implementations coming for the file_selector plugin example. I understand that the package name conflicts with something already in pub, and I think CrossFile is a good name for it; but could we maintain the name of the class that this package exports as (If we decide to go through with naming the class |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
This is a good point. I have renamed the classes (and file names) back to |
(I've pinged @jasonpanelli to get his approval!) |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@googlebot I consent. |
Ah yes, I forgot to mention this check:
|
Should be fixed now, CI is all green |
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.
This is going to be great. If there's any issue with the merge, feel free to add me to the CODEOWNERS for this package!
This reverts commit cf7e8d5.
Co-Authored-By: Jason Panelli <[email protected]>
ed352f5
to
481867c
Compare
Note to all: |
…le platforms. (flutter#3260) * Initial version of x_file package * Renamed from x_file to cross_file * Add back x_file type to file_selector * Fix formatting issues * Update homepage and version * Added README.md * Added missing copyright * Revert "Added missing copyright" This reverts commit cf7e8d5. * Add missing copyright Co-Authored-By: Jason Panelli <[email protected]> * Renamed class implementation back to XFile * Fix formatting issues * Rename to cross_file * Added code owners for cross_file package Co-authored-by: Jason Panelli <[email protected]>
…le platforms. (flutter#3260) * Initial version of x_file package * Renamed from x_file to cross_file * Add back x_file type to file_selector * Fix formatting issues * Update homepage and version * Added README.md * Added missing copyright * Revert "Added missing copyright" This reverts commit cf7e8d5. * Add missing copyright Co-Authored-By: Jason Panelli <[email protected]> * Renamed class implementation back to XFile * Fix formatting issues * Rename to cross_file * Added code owners for cross_file package Co-authored-by: Jason Panelli <[email protected]>
Description
This PR copies the x_file type from the file_selector package into its own package. This will allow us to easily reuse this cross platform file abstraction in other plugins that are dealing with files, without having to duplicate the code.
At the moment there is a requirement to use this type in the camera plugin to be able to add the web implementation (suggestion to use x_file originated from @ditman, see here). Additional interest has been shown to use it in the image_picker plugin and cache_manager package.
Note: I took the liberty of renaming
XFile
toCrossFile
since a package with the name "XFile" already exists on pub.dev but with a different purpose.If this PR is accepted and the package is published, I will make sure to also migrate the file_selector plugin to use this new package and delete the internal implementation.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?