Skip to content

Merge 18.8.0.4 beta into develop - #17652

Merged
mokagio merged 16 commits into
developfrom
merge/18.8-beta-4-into-develop
Dec 10, 2021
Merged

Merge 18.8.0.4 beta into develop#17652
mokagio merged 16 commits into
developfrom
merge/18.8-beta-4-into-develop

Conversation

@mokagio

@mokagio mokagio commented Dec 9, 2021

Copy link
Copy Markdown
Contributor

Got a bunch of conflicts because of deleted files on develop that this branch modified, most were localizations because of #17630 and #17636.

There was also a conflict on the change in the timeout duration of a test, which I solved by keeping the longest timeout value.

Other than the localizations, the conflicting files were

  • WordPress/WordPressTest/LikeUserHelperTests.swift
  • WordPress/Classes/ViewRelated/Me/App Settings/About/AboutHeaderView.swift
  • WordPress/Classes/ViewRelated/Me/App Settings/About/AutomatticAboutScreen.swift
  • WordPress/Classes/ViewRelated/Me/App Settings/About/AutomatticAppLogosCell.swift

I ensured the app built and the tests passed before committing 👌

Includes:

dvdchr and others added 14 commits December 8, 2021 16:44
This fixes an issue where the incorrect site was being shown after closing then reopening the app.
…ements-persist-chosen-site

Onboarding Improvements: persist the chosen site
…ckground-crash

Use derived context for WeeklyRoundup background task
Got a bunch of conflicts because of deleted files on `develop` that this
branch modified, most were localizations because of
#17630 and
#17636.

There was also a conflict on the change in the timeout duration of a
test, which I solved by keeping the longest timeout value.

I ensured the app built and the tests passed before committing 👌

Full list of conflicting files:

- `WordPress/Classes/ViewRelated/Me/App Settings/About/AboutHeaderView.swift`
- `WordPress/Classes/ViewRelated/Me/App Settings/About/AutomatticAboutScreen.swift`
- `WordPress/Classes/ViewRelated/Me/App Settings/About/AutomatticAppLogosCell.swift`
- `WordPress/WordPressShareExtension/ar.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/bg.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/cs.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/cy.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/da.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/de.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/en-AU.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/en-CA.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/en-GB.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/es.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/fr.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/he.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/hr.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/hu.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/id.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/is.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/it.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/ja.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/ko.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/nb.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/nl.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/pl.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/pt-BR.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/pt.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/ro.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/ru.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/sk.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/sq.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/sv.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/th.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/tr.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/zh-Hans.lproj/Localizable.strings`
- `WordPress/WordPressShareExtension/zh-Hant.lproj/Localizable.strings`
- `WordPress/WordPressTest/LikeUserHelperTests.swift`
- `WordPress/WordPressTodayWidget/ar.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/bg.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/cs.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/cy.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/da.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/de.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/en-AU.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/en-CA.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/en-GB.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/es.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/fr.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/he.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/hr.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/hu.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/id.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/is.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/it.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/ja.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/ko.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/nb.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/nl.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/pl.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/pt-BR.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/pt.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/ro.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/ru.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/sk.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/sq.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/sv.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/th.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/tr.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/zh-Hans.lproj/Localizable.strings`
- `WordPress/WordPressTodayWidget/zh-Hant.lproj/Localizable.strings`
@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Dec 9, 2021

Copy link
Copy Markdown

You can trigger an installable build for these changes by visiting CircleCI here.

@mokagio mokagio left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • Updates to the localization files (.strings and Fastlane metadata) automatically pulled for the new translations that have already been approved in GlotPress
  • Version update in .xcconfig
  • Diffs from the PRs that made it into this beta

Comment thread WordPress/WordPressTest/LikeUserHelperTests.swift Outdated
It became unused due to code changes in the merge conflicts resolution
from the previous commit, ea14d47
@mokagio
mokagio requested a review from a team December 9, 2021 05:48
@mokagio mokagio added this to the 18.8 ❄️ milestone Dec 9, 2021
@mokagio
mokagio marked this pull request as ready for review December 9, 2021 05:49
@mokagio
mokagio enabled auto-merge December 9, 2021 05:49
@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Dec 9, 2021

Copy link
Copy Markdown

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@frosty

frosty commented Dec 9, 2021

Copy link
Copy Markdown
Contributor

Thanks @mokagio! Just sharing some context about those About screen files: they have indeed been deleted, as after the code freeze they were replaced with a Swift package. I'm going to pull the changes from yesterday's PR into the packaged version today, so those changes won't be lost from develop before the next freeze.

@mokagio

mokagio commented Dec 10, 2021

Copy link
Copy Markdown
Contributor Author

😭 new conflict...

image

I'm going to solve it here, just in case someone gets to review it soon, even though I'll be running the release finalization in a few hours and possibly superseded this PR. I'd still have to solve the conflict in that case, so the learning is useful regardless.

@dvdchr

dvdchr commented Dec 10, 2021

Copy link
Copy Markdown
Contributor

@mokagio I can help review this in ~30 minutes. 🙂

There was a conflict in
`WordPress/Classes/ViewRelated/Blog/QuickStartPromptViewController.swift`

I solved by applying both the change from the release branch (
#17643) and
`develop` (#17648).
@peril-wordpress-mobile

Copy link
Copy Markdown
Messages
📖 This PR has the 'Releases' label: some checks will be skipped.

Generated by 🚫 dangerJS

@mokagio

mokagio commented Dec 10, 2021

Copy link
Copy Markdown
Contributor Author

@dvdchr thanks mate! That would be helpful 👍

@momo-ozawa can I also ask you to have a look at how I resolved the conflict in the quick start file here? 381fc14 (#17652)

@dvdchr dvdchr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested Quick Start according to the steps described in #17643. Found some oddities, but I suppose it's more related to #17648 rather than this merge conflict? Here's some 👀 on larger font sizes, tested on iPhone 8 Simulator, iOS 14.5, running the merge/18.8-beta-4-into-develop branch:

xxxLarge AX3 AX5
17652_xxxLarge 1652_AX3 17652_AX5
  • It looks like the updated serif label from #17648 doesn't scale with the font size category.
  • On AX5 size, the text seems to overlap with the action buttons, and I couldn't scroll down. Probably because it's not a scroll view?

Anyways, I'm not sure if this is a known issue or not. Since auto-merge is enabled, I'll defer to @momo-ozawa 🙂 . Hope this helped!

Comment on lines +81 to 83
promptDescriptionLabel.adjustsFontSizeToFitWidth = true

promptDescriptionLabel.textColor = .textSubtle

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is just a very minor, nitpicky note – but the new line is probably not intended here? Considering the textColor property belongs to promptDescriptionLabel. (Note that this totally is optional and shouldn't block the merge!)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like the updated serif label from #17648 doesn't scale with the font size category.

Ah. You're right :(

@mokagio I'll address the serif scaling in a different PR targeting develop.

On AX5 size, the text seems to overlap with the action buttons, and I couldn't scroll down. Probably because it's not a scroll view?

I'll embed the text in a scrollview, thanks for pointing that out.

@mokagio Which branch should I target for the scrollview issue? Wondering since the release for 18.8 is very soon.

@mokagio

mokagio commented Dec 10, 2021

Copy link
Copy Markdown
Contributor Author

Thanks a lot @dvdchr

@momo-ozawa momo-ozawa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The conflict resolution here makes sense to me. I'll address some of the issues @dvdchr found in a separate PR. Thank you @mokagio 🙇‍♀️

@AliSoftware AliSoftware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I looked specifically at the .strings file and AppLocalizedString.swift in the commit develop->merge-branch where you solved the issues and all seems good 👍

PS: I thought about also checking that the pbxproj merge got resolved properly and especially that the .strings files that were removed from disk were also removed from Xcode targets and that AppLocalizedString.swift were still properly added to the relevant targets… but I figured if Xcode builds without error on CI that's proof enough that those are correct, otherwise it would have failed on missing file references or unknown symbol.

@mokagio
mokagio merged commit 2fc7996 into develop Dec 10, 2021
@mokagio
mokagio deleted the merge/18.8-beta-4-into-develop branch December 10, 2021 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants