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

Docs: add recommendations on how to deprecate code #16433

Merged
merged 1 commit into from
Jul 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion docs/coding-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,21 @@ These are some things to keep in mind when writing code for Jetpack plugin. Plea
- Sanitize URLs, attributes, everything. WordPress.com VIP has this nice [article about the topic](https://vip.wordpress.com/documentation/vip/best-practices/security/validating-sanitizing-escaping/).
- Create [unit tests](https://github.com/Automattic/jetpack/tree/master/tests) if you can. If you're not familiar with Unit Testing, you can check [this tutorial](https://pippinsplugins.com/series/unit-tests-wordpress-plugins/).

## Deprecating code

When deprecating code in Jetpack (removing / renaming files, classes, functions, methods), there are a few things to keep in mind:

1. Other plugins / themes may be relying on that code, so we cannot just remove it. A quick way to gauge the use of a function can be to search for it in OpenGrok and [WPDirectory](https://wpdirectory.net/).
2. Deleting a file that was loaded and in use in the previous release can cause Fatal Errors on sites with aggressive OpCache setups.

For these reasons, here are a few guidelines you can follow:

- Instead of deleting files, mark them as deprecated first with `_deprecated_file`.
- Deprecate classes, functions, and methods in the same way, while still returning its replacement if there is one.
- Deprecated code should remain in Jetpack for 6 months, so third-parties have time to find out about the deprecations and update their codebase.
- If possible, reach out to partners who rely on deprecated code to let them know when the code will be removed, and how they can update.
- If necessary, you can publish an update guide on developer.jetpack.com to help people update.

## Widgets

- Make them support Customizer's Selective Refresh. Here's an [article about it](https://make.wordpress.org/core/2016/03/22/implementing-selective-refresh-support-for-widgets/).
Expand All @@ -34,4 +49,4 @@ Here are some general guidelines when considering adding new functionality:

- [Packages](../packages/README.md#should-my-code-be-in-a-package)
- Modules (@todo)
- module-extras.php (@todo)
- module-extras.php (@todo)
10 changes: 5 additions & 5 deletions docs/release-management.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

For every release, we will have a milestone that contains everything that is slated for that release. <strong>Issues/PRs should not be mass-punted from one release to the next.</strong> When getting punted from a release, issues/PRs should either have their milestone removed completely (from which they will need to be triaged), or they should be put into the "Not Currently Planned" milestone, which is any issue/PR that is valid, but not currently scheduled for inclusion in the point release or the next two major releases.

Any new feature/major bug fix going in gets tagged with "Master Issue" and either "[Type] Bug Fix", "[Type] Enhancement", "[Type] Janitorial" or "[Type] Feature". Initially, it should be tagged as "[Status] Proposal". If it's something that's absolutely required for this release, it gets tagged "[Priority] Blocker" so we know at a glance that it can't be moved to a future release. It's also tagged with its' area of the plugin as usual. The text of the master issue should be the proposed implementation, and that text should ALSO be posted as the first comment (more on this in a second)
Any new feature/major bug fix going in gets tagged with "Primary Issue" and either "[Type] Bug Fix", "[Type] Enhancement", "[Type] Janitorial" or "[Type] Feature". Initially, it should be tagged as "[Status] Proposal". If it's something that's absolutely required for this release, it gets tagged "[Priority] Blocker" so we know at a glance that it can't be moved to a future release. It's also tagged with its' area of the plugin as usual. The text of the master issue should be the proposed implementation, and that text should ALSO be posted as the first comment (more on this in a second)

If an issue is fairly involved, it may also make sense to create a GitHub project for it, and that project should be linked to from the “Master Issue”. Any sub-issues created to track portions of a master issue should be tagged “Subissue” and refer back to the master issue # as the first line in the issue body so that they can easily be filtered. Sub-issues should start at "[Status] In Progress".
If an issue is fairly involved, it may also make sense to create a GitHub project for it, and that project should be linked to from the “Primary Issue”. Any sub-issues created to track portions of a master issue should be tagged “Subissue” and refer back to the master issue # as the first line in the issue body so that they can easily be filtered. Sub-issues should start at "[Status] In Progress".

Any PRs would then get connected to that master issue, and conversation should take place within the master issue, with the <em>original post being regularly updated with the current state of things</em>, so that anyone can see, at a glance, where the issue stands, but they can read through the comments for more context. The Jetpack Lead and Release Lead should review any master issues for the upcoming release weekly (as part of their call) to make sure that the main post body is up to date. The last line of the main post body should always be “Last Updated: DATE"

Expand Down Expand Up @@ -38,12 +38,12 @@ The issue then gets updated as it moves through the process, from proposal to in
- <em>Low</em>


## Example Master Issue and Subissue formatting
## Example Primary Issue and Subissue formatting

### Master Issue
### Primary Issue

```
Synopsis: One or two sentences about what this Master Issue is aiming to achieve.
Synopsis: One or two sentences about what this Primary Issue is aiming to achieve.
Project: (Optional, if you are using a GitHub project to track the components of this MI, link to it here)
Expand Down