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

[RNMobile] Add block outline to all Social Link blocks when selected #6133

Conversation

derekblank
Copy link
Contributor

Related PRs

Testing Instructions

  1. Create a Post and add a Social Links block.
  2. Tap one of the four initial blocks (WordPress, Facebook, Twitter, Instagram) and note the blue Block Outline appears when the block is selected.
  3. Tap the [+] button to the right of the initial blocks to open the Social Link block inserter.
  4. Add any block from the inserter: Amazon, Dribbble, Dropbox, etc.
  5. When the new block is added, the blue Block Outline should appear around the new block. When de-selected by tapping anywhere else in the editor, and reselecting the block by tapping it, the blue Block Outline should appear.

@derekblank derekblank self-assigned this Aug 30, 2023
@derekblank
Copy link
Contributor Author

@dcalhoun I am unsure why wpmobilebot continues to close #6120 (other than the message "This PR is closed because there is no longer an associated gutenberg PR for it."). Is it because that PR is not assigned to anyone, perhaps? Re-opening this companion PR instead.

I wanted to confirm that I am following the correct sequence of steps for updating the companion PRs when the Gutenberg Mobile PR has a conflict with Gutenberg ref.

The steps I followed:

  1. Noted that [RNMobile] Add block outline to all Social Link blocks when selected #6120 and [RNMobile] Add block outline to all Social Link blocks when selected WordPress/gutenberg#54011 both had approvals
  2. Merged gutenberg#54011
  3. In Gutenberg Mobile, pulled the latest from trunk and merged it into version-toolkit/gutenberg/rnmobile/outline-all-social-link-block
  4. In that branch, updated the Gutenberg ref to trunk, which would also contain the merge of gutenberg#54011
  5. Wait for CI to pass in the Gutenberg Mobile PR and then merge it.

Let me know if there's a step you would have done differently. 🙇

Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

I am unsure why wpmobilebot continues to close #6120 (other than the message "This PR is closed because there is no longer an associated gutenberg PR for it."). Is it because that PR is not assigned to anyone, perhaps?

Yes. A cron job runs that will close any gutenberg-mobile PR whose sibling gutenberg PR is closed if the gutenberg-mobile PR does not have an assignee and it only contains commits authored by wpmobilebot. Additional information can be found in p9ugOq-2AI-p2.

Let me know if there's a step you would have done differently. 🙇

No, the steps you outlined are appropriate. In relation to avoiding auto-closure of the gutenberg-mobile PR, pushing the updated gutenberg ref commit in a timely fashion might've helped avoid erroneous auto-closure (not guaranteed, however, as the cron job could run at any moment).

However, I recommend assigning yourself to all of your PRs if your intention is to merge it yourself. Doing so communicates your intent very clearly all — bots or humans. Additionally, WordPress-Android and WordPress-iOS have differing approaches to merging PRs. The former expects PR reviewers to merge work unless an assignee is set. I am unsure how often this procedure is followed, but assigning yourself helps avoid confusion in many ways.

RELEASE-NOTES.txt Outdated Show resolved Hide resolved
Co-authored-by: David Calhoun <[email protected]>
@peril-wordpress-mobile
Copy link

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@derekblank
Copy link
Contributor Author

Thanks for confirming the merge process and for explaining the automated PR closure -- much appreciated. Good to know I was only missing the PR assignment step, which I agree is a good practice to follow for clarity.

@derekblank derekblank merged commit 4c3e251 into trunk Aug 30, 2023
@derekblank derekblank deleted the version-toolkit/gutenberg/rnmobile/outline-all-social-link-blocks branch August 30, 2023 23:09
@geriux geriux added this to the 1.103.0 (23.2) milestone Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants