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.

Sign up for GitHub

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

Jump to bottom

Merge 17.3 final into develop #16508

Merged
merged 14 commits into from
May 17, 2021
Merged

Merge 17.3 final into develop #16508

merged 14 commits into from
May 17, 2021

Conversation

jkmassel
Copy link
Contributor

@jkmassel jkmassel commented May 17, 2021 •
edited
Loading

Merges the last-second 17.3 fix and finalization into develop.

The reason for the huge drops in translatable strings is that we're no longer including certain Gutenberg strings.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 17, 2021 •
edited
Loading

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 17, 2021 •
edited
Loading

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 17, 2021 •
edited
Loading

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

Generated by 🚫 dangerJS

@jkmassel
Merge remote-tracking branch 'origin/develop' into merge/17.3-final-i…
9811669 
…nto-develop

# Conflicts:
#	Podfile
#	Podfile.lock
@jkmassel jkmassel self-assigned this May 17, 2021
@jkmassel jkmassel requested a review from a team May 17, 2021 04:44
@jkmassel jkmassel added this to the 17.4 milestone May 17, 2021
loremattei
loremattei reviewed May 17, 2021
View reviewed changes
Copy link
Contributor

@loremattei loremattei left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Is the removal of 1500+ strings expected? 🤔
Looking at the diff against trunk, I guess they were added by mistake, but I just want to double check that this is actually the case.

@fluiddot
Copy link
Contributor

fluiddot commented May 17, 2021 •
edited
Loading

Is the removal of 1500+ strings expected? 🤔
Looking at the diff against trunk, I guess they were added by mistake, but I just want to double check that this is actually the case.

Yeah, it's expected.

As a reference, here is the fix we did for removing the extra strings added by mistake (we removed 504 strings). For each string, we introduce three lines in the Localizable.strings files, so for removing 504, we're actually removing around 1512 lines from those files.

Screenshot 2021-05-17 at 13 43 14

@fluiddot fluiddot mentioned this pull request May 17, 2021
Merged
7 tasks
@jkmassel
Copy link
Contributor Author

Is the removal of 1500+ strings expected? 🤔
Looking at the diff against trunk, I guess they were added by mistake, but I just want to double check that this is actually the case.

Oh – yep – that's why I put "The reason for the huge drops in translatable strings is that we're no longer including certain Gutenberg strings." in the PR description, but perhaps I could've been more clear 😅

@oguzkocer oguzkocer merged commit cf5292c into develop May 17, 2021
@oguzkocer oguzkocer deleted the merge/17.3-final-into-develop branch May 17, 2021 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@loremattei loremattei loremattei left review comments

@oguzkocer oguzkocer oguzkocer approved these changes

Assignees

@jkmassel jkmassel

Labels
Releases
Projects
None yet
Milestone
17.4
Development

Successfully merging this pull request may close these issues.
6 participants
@jkmassel @fluiddot @oguzkocer @loremattei @ceyhun @jd-alexander