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

Reliable uploads - UPLOADS VIEW #1493

Merged
merged 225 commits into from
Mar 30, 2016
Merged

Conversation

davivel
Copy link
Contributor

@davivel davivel commented Mar 3, 2016

@owncloud/android-developers

Time has come.

From my POV, this is finally ready to be tested by QA (cc @jesmrec).

Many hands contributed here, starting with the awesome effort of @LukeOwncloud. Sounds wise summoning many eyes to review it. So, any comment about the code will be welcome.

Though many things have been rewritten and considerably cleaned-up, sure you will still find many points that could (and should) be refactored. I can't promise that all of them may be attended in this PR, since we really, really, need to release this with next version 1.9.2, that should be very near in time to the release of OC 9. Evolution of testing is the leading criteria right now.

This means: if we think we can refactor something in a safe manner without introducing an important risk for QA, we will do. If bugs appear and the code to fix is also subject of comments in the code review, we will apply the comments.

But if change requests are big enough to require QA starts the testing all over again, we will have to wait for a better chance.

This one should not be too far in time. This PR lets aside an important topic that was originally addressed by @LukeOwncloud , the improvement of automatic retries. Right now instant uploads are retried when Wifi connection is established, in a similar way to the operation of the currently released version 1.9.1 [1]. But other convenient improvements will be addressed specifically in the short term [2], not in this PR. And that should be a good moment to improve the code of uploads further.

Come on Barbie, let's go party!

[1] You may notice some light improvements, but that's a side effect of the many changes done; was easier let it that way that adding artificial limits. Have fun finding them :)

[2] If you don't want to wait and prefer to implement them, go for it. Just mention me to let me know, and I'll try to help.


BUGS FOUND AND ENHANCEMENT:

Luke Owncloud added 30 commits October 31, 2014 13:32
cleaning up FileUploadService.onStartCommand
preparing persistant uploads
check FileUploadService.java:

            //How does this work? Is it thread-safe to set mCurrentUpload here?
            //What happens if other mCurrentUpload is currently in progress?
            //
            //It seems that upload does work, however the upload state is not set
            //back of the first upload when a second upload starts while first is
            //in progress (yellow up-arrow does not disappear of first upload)
…ploads

Conflicts:
	src/com/owncloud/android/ui/adapter/FileListListAdapter.java
…ploads

Conflicts:
	src/com/owncloud/android/files/services/FileUploadService.java
	src/com/owncloud/android/operations/UploadFileOperation.java
	src/com/owncloud/android/ui/activity/FileDisplayActivity.java
Conflicts:
	src/com/owncloud/android/ui/activity/UploadFilesActivity.java
added menu for uploadListActivity
created separate ConnectivityActionReceiver
add comments
filter duplicate photos in InstantUploadBroadcastReceiver
@ghost
Copy link

ghost commented Mar 29, 2016

User of the commit aca6e66 is unknown - cannot determine CLA

@davivel
Copy link
Contributor Author

davivel commented Mar 29, 2016

I would remove the »delete« icon/function from the successfully uploaded files.

@jancborchardt , DONE now.

that reminds me that we should change the way the maximum of uploads is handled, it's being filtered in the view, while should be a maximum in storage.

DONE now

Any ETA on this branch?

@enoch85 , tomorrow in master, if @jesmrec agrees :)

Then, for 1.9.2, we need to fix #1514 , and enter in "release mode". Next week if we are lucky with the regression tests.

@ghost
Copy link

ghost commented Mar 30, 2016

User of the commit aca6e66 is unknown - cannot determine CLA

@ghost
Copy link

ghost commented Mar 30, 2016

User of the commit aca6e66 is unknown - cannot determine CLA

@ghost
Copy link

ghost commented Mar 30, 2016

User of the commit aca6e66 is unknown - cannot determine CLA

@jesmrec
Copy link
Collaborator

jesmrec commented Mar 30, 2016

Approved. Great job of all the android team & community!!!! Congrats

@davivel
Copy link
Contributor Author

davivel commented Mar 30, 2016

🐒 💃

@rperezb
Copy link

rperezb commented Mar 30, 2016

🏆

Conflicts FIXED in res/values/styles.xml (trivial)
@ghost
Copy link

ghost commented Mar 30, 2016

User of the commit aca6e66 is unknown - cannot determine CLA

@davivel
Copy link
Contributor Author

davivel commented Mar 30, 2016

Last merge from master to fix conflict was totally trivial.

👍 to all the code not written by me.

@malkomich , please, review code written by me.

@davivel
Copy link
Contributor Author

davivel commented Mar 30, 2016

@DeepDiver1975 , please, we need to remove the Required status of cla-bot-android until we can fix or prevent the current error.

@malkomich
Copy link
Member

👍

@ghost
Copy link

ghost commented Mar 30, 2016

User of the commit aca6e66 is unknown - cannot determine CLA

@davivel
Copy link
Contributor Author

davivel commented Mar 30, 2016

🎩

@davivel davivel merged commit b5a9594 into master Mar 30, 2016
@davivel davivel deleted the reliable_uploads_actions_uploads_view branch March 30, 2016 14:17
@AndyScherzinger
Copy link
Contributor

Aweseome! Kudos to everyone involved in this Mega-PR! 🎉 👏

@enoch85
Copy link
Member

enoch85 commented Mar 31, 2016

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.