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

Edit Post: Avoid rendering AdminNotices compatibility component #12444

Merged
merged 1 commit into from
Nov 29, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Nov 29, 2018

Alternative to: #12440
Related: #11604
Closes #12243

This pull request seeks to cease the admin notices compatibility upgrade. Unlike #12440, it keeps all of the code changes from #11604, but since all of the compatibility behavior had been implemented in an isolated component, it simply stops rendering the component, thereby avoiding its behavior.

Over #12440, it has some advantages:

  • Limited potential for breakage, since there is very little impacted code (simply removes the element from Layout's rendering)
  • Avoids the requirement of a major version bump on affected packages
  • Allows for / anticipates some future compatibility of admin notices (though not prescribing any specific implementation or UI treatment for how this would occur)

The downsides are limited to:

  • This is essentially dead code which lingers in the distributed packages as long as it remains unused
  • If we ultimately decide against any future compatibility, we either live with the dead code or a major version bump / breaking change inevitably must occur

Testing instructions:

Verify the inverse of the testing instructions outlined in #11604.

@aduth aduth added Backwards Compatibility Issues or PRs that impact backwards compatability [Package] Notices /packages/notices labels Nov 29, 2018
@mtias mtias self-requested a review November 29, 2018 21:29
@aduth aduth merged commit 512318a into master Nov 29, 2018
@aduth aduth deleted the remove/edit-post-admin-notices branch November 29, 2018 21:32
@mtias mtias added this to the 4.6 milestone Nov 29, 2018
daniloercoli added a commit that referenced this pull request Nov 30, 2018
…rnmobile/danilo-try-to-fix-undo-redo

* 'master' of https://github.com/WordPress/gutenberg:
  Autocompleters: Consider block category (#12287)
  Only init TinyMCE once per instance (#12386)
  RichText: convert HTML formatting whitespace to spaces (#12166)
  Notices: Remove "files" block in package.json (#12438)
  Edit Post: Avoid rendering AdminNotices compatibility component (#12444)
  Correct the docs manifest (#12411)
daniloercoli added a commit that referenced this pull request Nov 30, 2018
…HEAD

* 'master' of https://github.com/WordPress/gutenberg:
  [RNmobile] Fix problems with undo/redo on Android (#12417)
  Add registry param to withDispatch component (#11851)
  Autocompleters: Consider block category (#12287)
  Only init TinyMCE once per instance (#12386)
  RichText: convert HTML formatting whitespace to spaces (#12166)
  Notices: Remove "files" block in package.json (#12438)
  Edit Post: Avoid rendering AdminNotices compatibility component (#12444)
  Correct the docs manifest (#12411)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Package] Notices /packages/notices
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants