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

Google sheets component rebased #2261

Merged
merged 1 commit into from Jul 11, 2022

Conversation

tomiyee
Copy link
Contributor

@tomiyee tomiyee commented Jul 27, 2020

A Google Sheets Component to replace the (now discontinued) FusionTables Component.

Read Blocks are credential optional.
Write Blocks require that the developer create a Google Service Account and provide the credentials.json file to App Inventor.

@AppInventorWorkerBee
Copy link
Collaborator

Can one of the admins verify this patch?

@jisqyv
Copy link
Member

jisqyv commented Jul 28, 2020

@AppInventorWorkerBee ok to test

@halatmit
Copy link
Contributor

Let's make sure to test this thoroughly

@ewpatton
Copy link
Member

The latest version of this PR is available here: http://sheets.ai2-ewpatton-temp.appspot.com

@jisqyv
Copy link
Member

jisqyv commented Sep 2, 2020

Note: Because this PR is replacing libraries that are used by multiple components. Those components need to be identified and completely tested.

@jisqyv
Copy link
Member

jisqyv commented Sep 2, 2020

This PR contains an older and incompatible version of Evan's Multidex change. 8969d1e should be removed from this PR.

jisqyv
jisqyv previously requested changes Sep 2, 2020
Copy link
Member

@jisqyv jisqyv left a comment

Choose a reason for hiding this comment

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

8969d1e should be removed from this PR. This can be done via an interactive rebase (or using git rebase with the appropriate obscure arguments). I can also do this for you, provided I can push to your branch. Emacs "magit" has tools that makes this easy.

@ewpatton
Copy link
Member

ewpatton commented Sep 2, 2020 via email

@ewpatton ewpatton modified the milestones: September Release, nb187 Dec 1, 2020
@ewpatton ewpatton removed this from the nb187 milestone Aug 4, 2021
@jisqyv
Copy link
Member

jisqyv commented Sep 21, 2021

Not to add work, but...

All the Read and Write routines have a common pattern. The work to be done is set up, and then an asynchronous task is triggered to do the work that then calls a completion function on the UI thread.

Couldn't the asynchronous task be generalized and then used from all the various Read/Write calls (well, there may be one generalized task for reading and another for writing)? I believe in Java you can pass a method to call, so there can still be separate completion methods for each operation.

Just thinking...

@ewpatton ewpatton added this to the nb189 milestone Nov 22, 2021
@SusanRatiLane SusanRatiLane requested a review from a team April 28, 2022 17:17
@SusanRatiLane
Copy link
Contributor

@mit-cml/core-reviewers This should be ready for a new review.

@ewpatton ewpatton modified the milestones: nb189, nb190 May 10, 2022
@SusanRatiLane SusanRatiLane dismissed jisqyv’s stale review June 6, 2022 17:13

This should be done.

@jisqyv jisqyv self-assigned this Jul 8, 2022
@jisqyv
Copy link
Member

jisqyv commented Jul 8, 2022

So I'm going to do a detailed review and test of this component this weekend!

Co-Authored-By: Susan Rati Lane <srlane@mit.edu>

Change-Id: I2ac8e0eafa3556d56e52caaa97bfed9c39746be2
@jisqyv jisqyv force-pushed the google-sheets-component-rebased branch from 340af7d to a899dd5 Compare July 11, 2022 20:34
@jisqyv
Copy link
Member

jisqyv commented Jul 11, 2022

So, I rebased this change (again), squashed it down to one commit (signed). I also updated YOUNG_ANDROID_VERSION, I reverted the changes to Compiler.java, they were only whitespace. I fixed a document reference (removed the "ai2.appinventor.mit.edu part) so it works. I also checked in the updated documentation that results from a build. I also fixed the one merge conflict, which was trivial. After the unit tests run, and some last minute tests I want to run, I'll approve and merge this change!

Copy link
Member

@jisqyv jisqyv left a comment

Choose a reason for hiding this comment

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

LGTM

@jisqyv jisqyv merged commit a899dd5 into mit-cml:ucr Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants