Skip to content

Home

Jump to bottom
Koen Lageveen edited this page May 22, 2021 · 9 revisions

It is assumed that you know about the file formats for repository.json and dependencies, and how the latter can be included and should be structured.

Tip: Use a browser add-on that turns plain text URLs into clickable links for easier access to the referenced repository. E.g. Linkificator for Firefox.

Furthermore, I recommend keeping a list of standard replies handy.

Table of contents

Review guidelines

The following sections contain various procedures and things to look for in new or updated packages.

2x2 week timeout

After two weeks without a response, ping the PR author with a comment like @username ping and assign the stale label to the PR.

After another two weeks without a response, close the PR and assign the timeout label.

If the author does respond, remove the stale label and start over.

This procedure may not be repeated infinitely! We expect authors to be serious about their package and that they intend to maintain it.

Squash all the things

When merging, use the "squash and rebase" merge option in the merge button dropdown.

Reviewing a package addition

Review the pull request

  • Check if the tests are passing. Make the PR author aware of any issues. There may be scenarios where the tests are incomplete, so always review the changes briefly yourself.

  • The PR should contain no changes beyond the targeted package(s) (e.g. formatting the entire file).

  • Branch-based versioning is deprecated. Tags must be used in all scenarios. Examples are in the example repository.json.

  • Packages hosted on GitHub or Bitbucket should not provide "description", "issues", "readme" or "author" fields: they are taken from the repo directly.
    Even "name" can be left out unless it's different.

  • Search for similar packages on https://packagecontrol.io/. If there are any, refer to the Similar package(s) exist(s) section.

  • 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 the user does not have access, verify that the repo's owner has given permission, e.g. get an acknowledgement in the pull request via an @-mention.

  • Confirm that the package satisfies our package naming policies.
    Syntax packages should be named after the language, i.e. "LESS", not "LESS syntax" or "LESS highlighting" because syntaxes in ST do much more than just highlighting.
    Names should not be misleading regarding its functionality. e.g. .sublime-completions files do not provide "auto-completion", for example.

Review the package itself

  • A README is required, otherwise people won't know what the package does, and should be understandable for new users. It should probably also be in English, except where it obviously shouldn't.

  • Check that the repository has valid semantic version tags, with the appropriate prefixes if needed.

  • Check for specific files in the repository:

    • Media files used in the README, like images, videos, can be excluded from the package using export-ignore in a .gitattributes file.

    • *.pyc, *.cache binary files are auto-generated by Sublime Text.
      Unless the package does not ship the .py source files and is distributed in a "closed source" form, such files must be removed.

    • package-metadata.json must not be present!

    • If messages.json is present, check if it is correctly formatted (i.e. valid JSON, uses correct keys, specified files actually exist). (docs)

    • Check if dependencies.json (and the dependencies specified) is valid and does not include arbitrary dependencies from pypi, for example.

    • The .no-sublime-package file should not used since it complicates overriding and modifying packages locally unless:

      • 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.
      • The plugin code loads binary Python modules.
      • 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. If a module uses numerous absolute imports to import parts of itself and an addition to sys.path is necessary, this also works if the package is archived because of Python's internal ziploader.

Review the source code

  • All commands should be discoverable via the command palette, usually in a Default.sublime-commands file.

  • Commands should not be added to the context menu, to prevent clutter. Unless they clearly enhance usability for all users. Context menu entries should be removed from the menu (e.g. via is_visible) if not applicable to the context, not simply disabled (via is_enabled).

  • Commands in the main menu need to be in the correct section. New top level menus should not be created.

  • Keybindings are not recommended: there are not enough keys on the keyboard to support all packages. Also, people can't remember them all anyway.
    Recommend not shipping keybindings in the package, instead documenting how users can create them in the README. And ensure the commands are in the palette, which in most cases is as good as a keybinding.

  • Python plugins that use ST's API must be in the root folder of the repository. Otherwise Sublime Text will not find them.

  • Packages are delivered in an archive format, so files in it cannot be reached using open.

  • The package name, as set in the package control repository, is relevant for opening settings files etc. If it differs from the repo name, ensure references in code are to the correct name.

  • For packages that can be easily installed and tested, ensure they don't impact performance, e.g. by doing a lot of work on keystrokes or when creating menu entries.

  • If a package does not define Python plugins, it is generally compatible with all OSes.

  • Advanced:

    • Verify that all (file) handles are closed. This is especially important with file locks on Windows.
    • Check if package satisfies its install requirements compatibility-wise. (Does the code work on all platforms? Does it perhaps work on more than specified?).
      If the package is cross-platform, check if it hard-codes forward slashes for file paths or assumes this to be the case.
    • Check whether a package is bundling dependencies that are also available as Package Control dependencies.
    • Check for unnecessary usage of sys.path and suggest using relative imports if possible.

Reviewing a package change

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 package rename, (change of name attribute in package data), check if the old package name was added to the previous_names list.

  • For a repository rename/move, check if the old repository URL redirects to the new one. GitHub allows renaming a repository, which adds HTTP 301 redirects to old URLs and preserves issues and pull requests.

  • For a user rename, check that the old user (or repo) URL (e.g. http://github.com/OldFichteFoll) redirects to the new URL. This confirms that it was indeed a user rename and not a different user.

  • For a repository takeover, confirm that the original repository owner agrees to the takeover. For instance, mention the repository owner directly and also mention a two week deadline.
    After two weeks or more you may merge the takeover.
    For huge functionality changes, suggest to add an update message for the takeover.

Reviewing a dependency addition

  1. Review the pull request similar to a package addition.

  2. Assert that the folder structure is valid (modules inside all/, st3/ or st3_linux_x64/ sub-folders).

  3. If the repository contains loader.py, review it and use your best judgment (has not occurred yet).

Reviewing a repository addition

  1. If the package(s) contained in the repository are hosted on a supported provider (github, bitbucket) and do(es) not require special bundling, suggest to add the package(s) to the PCC repo directly. This has the advantage of easy batch-updates of packages.

  2. As always, verify that the tests pass and review the changes briefly.

  3. If you are unsure about the validity of the external repository file, run it through PackageRepositoryTools's "Test Remote Repository" command.

  4. Check if the URLs specified 404.

Similar packages

We try to keep duplicated packages at a minimum because of several reasons, mainly:

  1. Users will have to choose between several packages that all achieve practically the same thing, usually without a clear indication which will be best for them.
  2. Package authors or maintainers are also effectively doubling their work (of maintenance). This can be prevented by working together on the same package (either by collaborating or by proposing pull requests).
  3. Having more contributors on one package increases it's long term life expectancy.
  4. Some times packages stop being maintained. Replacing those with a new package for the same utility keeps package control fresh.

If one or more similar packages exist, it's always recommended consolidate them and enhance an existing packag.

When in doubt, mention the author of a similar package and ask for their judgment or willingness for cooperation.

When the packages' use cases are different enough, or the authors are unable to come to come to an agreement, you may merge the package anyway.

Standard replies

Tip: add these replies to a clipboard manager of choice, e.g. ArsClip.

You have to push a valid [semantic version](http://semver.org) [tag](http://git-scm.com/book/en/v2/Git-Basics-Tagging) (with optional leading `v`) to your repo.  
[Semantic versions](http://semver.org) consist of exactly three numeric parts.
Branch based releases are deprecated.
Note that, by default, pre-release versions are not installed by Package Control. This has to be enabled in its settings before attempting to install your package. I suggest releasing an non-prerelease version.
When renaming a package you have to add the old name to a `previous_names` array.
You'll need to change `"branch": "master"` to `"tags": true` and use [tags](http://git-scm.com/book/en/v2/Git-Basics-Tagging) to create releases and push valid [**semantic version**](http://semver.org) tags to your repo. As mentioned [in the docs](https://packagecontrol.io/docs/submitting_a_package#Step_4), we no longer accepts branch-based releases for new packages.
The tests don't pass: https://packagecontrol.io/docs/submitting_a_package#Step_7
SublimeLinter plugins should be submitted to [their repository](https://github.com/SublimeLinter/package_control_channel).
By convention, your package is providing "completions" and not "auto-completions", because those would be context-aware (e.g. local variables and/or type awareness). Please adjust the label.
Clone this wiki locally