Consider adding my "TinifyImage" package

[thewebprojects]

Hello,

I'm seeking a review of my custom Sublime plugin (as if you didn't already know that ;)

Please bear with me. This is my first Sublime plugin, my first Python project, my first time dealing with Git and GitHub. Yeah... I'm old school, trying to get up to speed on this stuff.

• I'm the package's author and/or maintainer.
• I have have read [the docs][1].
• I have tagged a release with a [semver][2] version number.
• My package repo has a description and a README describing what it's for and how to use it.
• My package doesn't add context menu entries. *
• My package doesn't add key bindings. **
• Any commands are available via the command palette.
• Preferences and keybindings (if any) are listed in the menu and the command palette, and open in split view.
• If my package is a syntax it doesn't also add a color scheme. ***
• If my package is a syntax it is named after the language it supports (without suffixes like "syntax" or "highlighting").
• I use [.gitattributes][3] to exclude files from the package: images, test files, sublime-project/workspace.

There are no packages like it in Package Control.

If there's anything I can do to improve this plugin please advise.

Thank you so much for your time.

[packagecontrol-bot]

Automated testing result: SUCCESS

Packages removed:
  - TinyScheme Auto-Complete

[braver]

why are you changing this other package as well?

[thewebprojects]

Any changes to this other package was a mistake.

[FichteFoll]

Please avoid vendoring Python libraries and making them accessible to the entire plugin host via sys.path modification when possible. We have requests available as a dependencylibrary already, although it is an older version (latest that supports Python 3.3) and hasn't been updated for the 3.8 host yet. Since the plugin host environment is shared among all plugins, having multiple requests libraries is bount to cause issues and shadowing because only one such module will be loaded and is then cached. Furthermore, I strongly suggest to add tinify as a library to https://github.com/packagecontrol/channel/ as well because it will make distribution easier as you can then also remove your .no-sublime-package file. Usually it'd be fine to vendor it since it uses relative imports, but it does some weird singleton thing with sys.modules that's, again, bound to cause issues if multiple such libraries exist in the same Python environment.

[kaste]

You MUST use requests as a dependency. Basically all ST users have that already installed, as they have some plugin using it. And we can't have version conflicts for that. Therefore certifi, idna at least go away as well. Dependencies are declared in a dependencies.json file. You can see that in the docs.

.pyc files are typically not checked into git. All unnecessary files need to be excluded from export using a .gitattributes file.

As the command itself calls the internet for each selected file. I think you should hop onto the worker thread at some point by scheduling a task (callable) using sublime.set_timeout_async.

Ideally you wouldn't alter sys.path or sys.module and just call the API using requests. The tinify "lib" is in itself so tiny, you could just very likely do the calls directly.

[kaste]

Oh, hi @FichteFoll. 😁 You beat me by a minute.

[thewebprojects]

@FichteFoll, @kaste,

Thanks for taking the time to review the files and offer your input.

So it seems like the preferred approach is to add tinify to https://github.com/packagecontrol/channel/ and use dependencies.json to request these libraries instead of self-hosting anything. That would be a tidier solution.

[deathaxe]

Packages should also avoid deploying __pycache__ folders or any other auto-pre-compiled *.pyc files as those are a compatibility issue for sure, once ST may decide to support newer python versions and those are bloating packages.

[kaste]

So it seems like the preferred approach is to add tinify to packagecontrol/channel and use dependencies.json to request these libraries instead of self-hosting anything. That would be a tidier solution.

@thewebprojects For requests and its dependencies, you MUST use the library on packagecontrol. For tinify itself, @FichteFoll suggested adding it to packagecontrol as well, and I would just not use it and use only requests to call the API directly (as it is just too simple in my opinion).

Some paths, like __pycache__ or *.pyc, typically don't get checked in into a git repo and thus should be ignored using a .gitignore file. (E.g. https://github.com/timbrel/GitSavvy/blob/eb70249e724f933255b4d8c7096092f2764942b4/.gitignore) Other files you want to check in but want to prohibit that end-users have to download them. These land in .gitattributes files. (E.g https://github.com/SublimeLinter/SublimeLinter/blob/029ff3dcf441f5606ecb329d66d76ed7df601e96/.gitattributes)

[thewebprojects]

Say I update my plugin/repo, do I then report back here? What's the protocol? Please advise.

[kaste]

Yes, update and report back. Or ask questions.

[thewebprojects]

I'm going to switch gears and attempt to build a ST plugin with the Pillow SIMD library, and not use the Tinify API. Reasons: I'm new to PY and there's no detailed documentation on how to use PY with the Tinify API without their library. Furthermore, my TinifyImage plugin requires the user get a Tinify API key which most folks are not interested in doing. I just submitted a pull request for adding PIllow SIMD to Package Control Libraries Channel.

With this change of direction the name TinifyImage isn't appropriate for this plugin. What would you say is the most appropriate course of action now? Do I just remove the TinifyImage repo or attempt to rename it and change all the files? Bear with me as I'm learning the ropes with ST plugin development, Python AND use of Git and Git Hub. Ugh.

Thanks!

[kaste]

Yeah, you need to make that Pillow PR "against" the original repo not on on your fork. (Because PR is a pull request, on your fork you just merge whatever you want, but here you want @deathaxe to pull your changes so we all see them.)

You could rename your repo but you can also just start a new one. That doesn't matter at all. That being said, I don't think Pillow-SIMD ships compiled, ready to use wheels or whatever. It needs to be compiled on your machine. Compilation is considered out of scope for package control. Maybe switch to Pillow sans SIMD (https://pypi.org/project/pillow/#files).

[thewebprojects]

Oh boy. I'm a mess. @kaste, thanks for your insight and patience.

[kaste]

@thewebprojects No need to change careers 😄, happens to all of us. 👍