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

Merge develop to master auto upload #10524

merged 289 commits into from Sep 25, 2019


Copy link

oguzkocer commented Sep 25, 2019

This PR merges develop into the master-auto-upload branch. There were some merge conflicts and since I don't have any context into the changes in develop or master-auto-upload, I thought I'd document them as a PR.

Simple conflicts

  1. build.gradle: Only conflict was related to FluxC hash. Instead of fixing the conflict, I just updated the hash to the latest for master-remote-auto-save FluxC branch which recently got develop merged into it.
  2. AnalyticsTracker event names
  3. AnalyticsTrackerNosara switch cases

Somewhat tricky conflicts

  1. PostUploadHandler: d2f471f removed onMediaUploaded and handleMediaUploadCompletedLegacy in develop - master-auto-upload added onPostChanged.
  2. EditPostActivity: c02c316#diff-38b6aea9cfff5f3e0e4b7389b44de602L1959 removes setLegacyMode call in develop - master-auto-upload moves the tracking to the UploadService
  3. PostListButtonType: #10441 changed the color resource to be an attribute in develop - master-auto-upload added 2 new types. BUTTON_DELETE_PERMANENTLY's color was updated to wpColorTextSubtle directly and BUTTON_CANCEL_PENDING_AUTO_UPLOAD's color was added manually as a temporary solution since it's already being added in #10496. This will create another (very easy to solve) merge conflict when we or #10496 is merging to develop whichever happens later.

To test:

Since this is just a merge PR, we don't need to test anything specific here except for making sure the build works fine. We'll be running a full set of tests in the master-auto-upload branch after this merge.

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.
khaykov and others added 30 commits Aug 14, 2019
This flag will determine if dialog will be dismissed after confirm is clicked. It's default behavior is to dismiss after confirm since the existing implementations utilize this.

Added dismissAfterConfirm flag to FullScreenDialogFragment
…ality can be shared.

The class now has the common functionality that will be needed in the sign-up and the account settings flow. The behaviour that differs with the header & analytics will be implemented in the respective classes that extend it.
…d Injector method since it makes it easier to test.

The FragmentInjector can be replaced during tests. The older method requires modification to the TestComponents/TestModules etc. Also corrected how injections were taking place by moving injection to the subtype instead of the base type since the best practice is to inject subtypes.
Verifies that the suggestions functionality is still intact even though the implementation has been modified. Created an environment where the fragment could be tested in isolation utilizing Mockito,Dagger's Android Injection and a fragment container.
jtreanor and others added 13 commits Sep 13, 2019
Update to gutenberg-mobile 1.13.0

Add release notes for local autosave from #10483

Remove hybrid and legacy editor, and related code

Feature/move me tab to new activity

As part of merging `develop` into `master-auto-upload` branch, we need
to fix a conflict for `PostListButtonType` by changing the
`R.color.warning_dark` to an attr. This attr didn't exist yet in
`develop` but was already added in #10496. As a temporary workaround it
was suggested to add the necessary attr.
@oguzkocer oguzkocer added this to the 13.4 milestone Sep 25, 2019
@oguzkocer oguzkocer requested review from shiki and malinajirka Sep 25, 2019
@oguzkocer oguzkocer added this to In Review in Offline Support: Posting [Android] via automation Sep 25, 2019

This comment has been minimized.

Copy link

peril-wordpress-mobile bot commented Sep 25, 2019

⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS


This comment has been minimized.

Copy link

peril-wordpress-mobile bot commented Sep 25, 2019

You can test the changes on this Pull Request by downloading the APK here.


This comment has been minimized.

Copy link
Contributor Author

oguzkocer commented Sep 25, 2019

The connected tests were broken by some changes in FluxC. I don't have any experience with WPAndroid connected tests, but it looks like df41912 has fixed it.

@shiki shiki self-assigned this Sep 25, 2019
shiki approved these changes Sep 25, 2019
Copy link

shiki left a comment

@oguzkocer Thank you for the detailed description of the conflicts and how you resolved them. I reviewed this by going through the merge myself and comparing develop, master-auto-upload, and this branch. It looks good to me.

  • AnalyticsTracker
  • AnalyticsTrackerNosara
  • PostUploadHandler
  • EditPostActivity
  • PostListButtonType


@shiki shiki merged commit c8737ba into master-auto-upload Sep 25, 2019
6 checks passed
6 checks passed
Peril Found some issues. Don't worry, everything is fixable.
ci/circleci: Installable Build Your tests passed on CircleCI!
ci/circleci: connected-tests Your tests passed on CircleCI!
ci/circleci: lint Your tests passed on CircleCI!
ci/circleci: strings-check Your tests passed on CircleCI!
ci/circleci: test Your tests passed on CircleCI!
Offline Support: Posting [Android] automation moved this from In Review to Done (PRs) Sep 25, 2019
@shiki shiki deleted the merge-develop-to-master-auto-upload branch Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.