Process for reviewing packages on the package_control_channel repo

[FichteFoll]

I'll use this ticket as a place to draft the process for reviewing packages. It will be updated over time when I feel like it. Eventually this should be added to packagecontrol.io. This will allow us to introduce potentially more helpers to the process and can also serve as a checklist for experienced reviewers (such as myself) in case I forget something.

---

It is assumed that you know about the repository.json file format. See example-repository.json.

Reviewing a package addition to the default Package Control channel

1. Review the pull request.
   1. Check if the tests are passing. If they are not, make the PR author aware of the issue and potentially link to the error line on travis-ci. Beware that there may be scenarios where the tests are incomplete, so always review the changes briefly by yourself.
   2. Check if there are no commits that modifiy an entire file and have 1000+ lines changed. It will completely mess up the history. Request the PR author to fix this by squashing/reverting/opening a new PR.
   3. As of Package Schema 3.0.0, branch-based versioning is not accepted anymore for new packages. They must use tags, even for multi-branched releases for separate ST2 and ST3 versions. There is an example for this in the example-repository.json file.
   4. Confirm that the user who authors the pull request has push access to the repo he wants to add. For an organization, check if the user is part of that organization or check if he pushed commits to the repo. If he does not have access, see if the repo's owner has given permission to adding the package to Package Control's default channel or get his acknowledgement in the pull request by mentioning him.
   5. Confirm that the package satisfies our package naming policies.

2. Review the package.

   1. Read the Readme and assure that you somewhat understand what it does. If there is no readme or you don't understand it (e.g. it's written in a language you don't understand), ask the PR author to add a readme, translate it to English (if the package has an international audience) or ask for an explanation.
   2. Search for similar or duplicate packages on https://packagecontrol.io/. If there are any, see below for the "similar packagejs) exist(s)" section. Be smart in your search terms or remember adding a similar package previously.
   3. Check that the repository has correctly formatted semantic version tags with the appropriate prefix specified in the PR. If "tags": true, both no prefix and v will be recognized currently. (https://github.com/wbond/package_control/issues/887)

   4. Check for specific files in the repository:

      1. *.pyc, *.cache binary files are auto-generated by Sublime Text and stored in the same folder in ST2. Unless the package does not ship the .py source files, request the author to remove those files.

      2. The .no-sublime-package file should only be used if really necessary since it complicates overriding and modifying packages locally. Such a file is needed if

         • The plugin code loads binary Python modules.
         • The package executes binaries included in the package itself using the subprocess module.
         • The package runs scripts included in the package using an external application, such as node.
         • A bug in Sublime Text prevents a certain feature to work if in archived state. (none known currently)

         It is not needed for bundling Python modules. Relative imports work in both ST2 and ST3 (although differently for ST2 in the package's root folder). If a module uses numerous absolute imports to import parts of itself (grrr) and an addition to sys.path is necessary, this should also work if the package is archived. TOTEST.

      3. package-metadata.json should not be present! Ask the author to remove it.
      4. If messages.json is present, check if it is correctly formatted (i.e. valid JSON, uses correct keys, specified files actually exist). (docs)
      5. Python plugins that use ST's API must be in the root folder of the repository, otherwise Sublime Text will not find them.
   5. If a package defines no Python plugins, it is generally compatible with all OSes. ST2&3 compatability may depend but is usually the case. Point out if the pull request's requirements are too restrictive.
   6. Optionally, review the source code of Python plugins.
      1. TODO
         major: close all (file) handles; check if the package considers being an archived package on ST3; check if package satisfies its install requirements compatability-wise (does the code work on ST2/3? Windows? does it perhaps work on more than specified?); try to guess if a plugin is taxing on performance
         minor: don't hardcode forward slashes for file paths; point out general flaws in code or give tips
3. If package is a dependency:
   1. TODO

      Reviewing a package change in the default Package Control channel

Generally the same steps as for when adding a package apply, except that you don't need to review the package since it has already been done when it was added. However, there are a few additional things to consider in certain situations.

• For a repository rename, check if the old repository url redirects to the new. If it does not, mention that github has a feature that allows renaming a repository that adds 301 redirects and preserves issues and pull requests.
• For a user rename, check that the old user (or repo) url http://github.com/OldFichteFoll redirects to the new user or repo url. This confirms that it was indeed a user rename and not a different user.
• For a package rename (change of name attribute in package data), check that the old package name was added to the previous_names list.

  Similar package(s) exist(s)

TODO

Cleaning up old pull requests

TODO

---

cc @wbond, @joneshf

Since I'll be away from 27th thru 8th it sounded like a good idea to do that now.

[joneshf]

Excellent! Thanks!

[FichteFoll]

How do we feel about encouraging to use the naming conventions for color schemes or themes? Both seem to have two "implicit" conventions, which are:

• Color Scheme - <name> and <name> Color Scheme, or
• Theme - <name> and <name> Theme respectively.

I mean, we have similar implicit conventions of naming a package <language name> Snippets or <language name> Completions if they only contain that.

[joneshf]

I have no strong preferences either way. It looks like there are more <name> Color Scheme and more Theme - <name> than either respective alternative.

[FichteFoll]

This has been moved to PCC's wiki (with actual version history and probably better discoverability): https://github.com/wbond/package_control_channel/wiki/Pull-Request-Review-Guidelines

I also updated it in the mean time.