update pieces package

[bishoy-at-pieces]

• I'm the package's author and/or maintainer.
• I have have read the docs.
• I have tagged a release with a semver 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 to exclude files from the package: images, test files, sublime-project/workspace.

My package is for interacting with Pieces OS.
I updated the old package that was no longer maintained and added this which is maintained by Pieces.app

[bishoy-at-pieces]

@packagecontrol-bot

[braver]

Thanks. What happened to @crispieces who originally added this package?

[packagecontrol-bot]

Automated testing result: SUCCESS

Packages modified:
  - Pieces

[bishoy-at-pieces]

Thanks. What happened to @crispieces who originally added this package?

Not sure actually but that pieces package no longer works nothing really works and it is not maintained. Last edit was from 2 years.

[bishoy-at-pieces]

Is this ready to go @braver ?

[braver]

Right, we did have an episode about Pieces a while ago: #8508. So this looks like yet another restart of the same project? And a much more complicated package than the one it's replacing. We need to give the owner of the original package two weeks to let us know if it's ok to replace his package or not. And we'll review the package itself as well.

[kaste]

I throws

WARNING:urllib3.connectionpool:Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<urllib3.connection.HTTPConnection object at 0x00000218A32A4430>: Failed to establish a new connection: [WinError 10061] Es konnte keine Verbindung hergestellt werden, da der Zielcomputer die Verbindung verweigerte')': /.well-known/version

after installing. EDIT: Because it throws, I unfortunately cannot test this package any further.

You install global key-bindings which is highly discouraged. (E.g. https://github.com/pieces-app/plugin_sublime/blob/main/keybindings/Default%20(Windows).sublime-keymap) The standard behavior of ctrl+e is broken just by installing the package. 😢

EDIT: I didn't get the install notes (RTFM! 😁) as I'm obviously have to install manually before it gets on Package Control. So I have to install a bigger app/software to get this working. Sorry for the noise.

[bishoy-at-pieces]

I throws

WARNING:urllib3.connectionpool:Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<urllib3.connection.HTTPConnection object at 0x00000218A32A4430>: Failed to establish a new connection: [WinError 10061] Es konnte keine Verbindung hergestellt werden, da der Zielcomputer die Verbindung verweigerte')': /.well-known/version

after installing. EDIT: Because it throws, I unfortunately cannot test this package any further.

You install global key-bindings which is highly discouraged. (E.g. https://github.com/pieces-app/plugin_sublime/blob/main/keybindings/Default%20(Windows).sublime-keymap) The standard behavior of ctrl+e is broken just by installing the package. 😢

EDIT: I didn't get the install notes (RTFM! 😁) as I'm obviously have to install manually before it gets on Package Control. So I have to install a bigger app/software to get this working. Sorry for the noise.

You don't have pieces os

[kaste]

Yeah, working. I think you should rethink how you do the global key bindings, as you can't just grab right-delete or ctrl+e. You probably only need them under certain circumstances or for specific views, t.i. generally in a specific context. This context should be reflected in the bindings, that means: they should only bind for some views, or in some modes of the editor.

[bishoy-at-pieces]

Yes I know, but html sheet does not support context. As for now I will leave it like that but probably adding buttons in the html sheet instead of keybinding later. There are are many other functionalities that I am working on this is not essential for now.

[mark-at-pieces]

@braver

Hope all is well. A couple years ago an intern of ours added the v1 of Pieces.

Since then a lot has changed.

We are an organization around 20-25 employees, and would love to know how our organization can verify ownership of the project rather than ownership given to an individual user. So that we can mitigate these issues in the future.

As of right now, the previous project was in our previous organization on Bitbucket but we are now (as of the last 1-2 years) fully on Github, however I have full access to all the projects in our previous org, so I can verify ownership of the project.

Please let me know anything that I can do to help speed up the progress here, as well as anything that I can do on my side to mitigate these issues in the future.

Thanks for your help on this!!🤘

[kaste]

Please make it clear in the README, right at the top 😁, that the plugin requires a different app to be installed as well. You do this in "install.txt" but you ask to publish it on Package Control and then it gets advertised using your README. This should be clear before installing, and really just by glancing at it, esp. as it is not that usual.

I already said that the global key bindings are problematic. You clearly grab very prominent shortcuts, e.g. ctrl+s, ctrl+e, delete. And I already said that ctrl+e is in fact broken for every view just by installing this plugin. That would maybe be okay if you grabbed them because they were applicable on every view. But they're not, ctrl+e just does nothing now. I think you should invest some time to do a workaround here. Is the generic settings-context (settings.the-foo) really not called/queried? the-foo can technically be any view setting, e.g. anything defined in Preferences or in a project or per syntax. Otherwise, maybe make them opt-in.

You also claim space in the context menu. Typically there should be a way to opt-out, or opt-in of course. How to do that should be in the README.

Q: Have you actually tried not setting the ids, e.g. esp. "id": "pieces",. Typically the placement of the items within the context menu is affected by this. Where is the menu entry inserted on your PC? Is setting an id a tip coming from discord?

Please note that you did not check the bullets in the questionnaire regarding these two points exactly, so you'd be prepared for us asking about them. Everything else seems ok to me. 👍

[kaste]

Ah, I totally forgot about the dependencies. Can you please use dependencies.json for these. (Is there actually cleanup code for the dependencies on uninstall? I don't think so.) See https://github.com/packagecontrol/channel/blob/main/repository.json for what we already have. pydantic should be in there for example, and if so you MUST use it. Ideally you vendor the esoteric stuff, t.i. don't add it to sys.path at all. Common packages should be added to that channel though.

Ref: https://github.com/pieces-app/plugin_sublime/blob/main/__init__.py
Ref: https://github.com/pieces-app/sublime-dependencies

[bishoy-at-pieces]

Ah, I totally forgot about the dependencies. Can you please use dependencies.json for these. (Is there actually cleanup code for the dependencies on uninstall? I don't think so.) See https://github.com/packagecontrol/channel/blob/main/repository.json for what we already have. pydantic should be in there for example, and if so you MUST use it. Ideally you vendor the esoteric stuff, t.i. don't add it to sys.path at all. Common packages should be added to that channel though.

Ref: https://github.com/pieces-app/plugin_sublime/blob/main/__init__.py
Ref: https://github.com/pieces-app/sublime-dependencies

So for this we always update our sdks and it won't be optimal to vendor it since all imports are not relative imports so I will stick with the current implementation for the dependencies download but for the keybinding I will be changing it no worries in the next release I added an issue with this. thank you

[kaste]

The way you handle dependencies is a blocker. Didn't you noticed the uppercased "MUST"? You also install dependencies in a folder you don't own, and you clearly don't even attempt to uninstall. Otherwise just point me at the code. Was this approach
specifically suggested to you on Discord?

As this is baked by a company: Is there any telemetry? If so it MUST be opt-in. Also make it clear if you send data when installing the plugin or by viewing/opening a file. Typically this information should also be rather at the front of the README.

Please address the feedback and report back.

[bishoy-at-pieces]

I can't see any issues in installing the dependencies myself instead of the package control since the sdks not in the package control and everything is clear in the README, not also sure why you are referring to "discord" in everything. I simply though about this since the sdks imports are absolute imports, and I can't use sys.path

[bishoy-at-pieces]

You can check the code no data is collected! I think the code is clear enough

[mark-at-pieces]

@braver

Hope all is well. A couple years ago an intern of ours added the v1 of Pieces.

Since then a lot has changed.

We are an organization around 20-25 employees, and would love to know how our organization can verify ownership of the project rather than ownership given to an individual user. So that we can mitigate these issues in the future.

As of right now, the previous project was in our previous organization on Bitbucket but we are now (as of the last 1-2 years) fully on Github, however I have full access to all the projects in our previous org, so I can verify ownership of the project.

Please let me know anything that I can do to help speed up the progress here, as well as anything that I can do on my side to mitigate these issues in the future.

Thanks for your help on this!!🤘

@braver wanted to ping this message over again, hope you had a great weekend, looking forward to hearing from you.

[kaste]

How about just addressing the feedback already given, @mark-at-pieces? 🤴

[bishoy-at-pieces]

Thank you @kaste for your feedback it is really helpful and all of it is changed as you asked!
keybindings (will be merged next release): https://github.com/Bishoy-at-pieces/plugin_sublime/commit/e95539980d157c20ee5d500936abd255932af46d
Edit the README to show that you need to install Pieces OS: pieces-app/plugin_sublime@a2021c5
Removed the context id:pieces-app/plugin_sublime@bdb76c6

[kaste]

The general mindset should be that you're in shared environment. That goes for key bindings and the context menu, but also for sys.path, sys.modules, and sys.excepthook(). For key bindings and context menus, there should be opt-in or opt-out. And you always have options, that's why I asked you about Discord. If you have id set for an menu item, it behaves differently: a) it should look slightly different, and b) you have some other ways to opt-out.

As I said, you MUST do the dependencies differently. Lib\python38 is not your folder, and you cannot install and for example never uninstall whatever you want. This folder is controlled by Package Control. a) you MUST use libraries listed in https://github.com/packagecontrol/channel/blob/main/repository.json and just declare them in dependencies.json. Otherwise we have potentially conflicting libraries in sys.modules. b) Common python libraries can be added to the same channel/repository. Just open a PR over there, so that all users benefit from it. c) You can also ask to add esoteric libraries, e.g. your pieces-lib to this channel, FichteFoll suggests that, or you vendor it, typically I propose that. d) If you need specific versions (unlikely) of a library, you MUST vendor. e) Vendoring can be done in two ways: This is explained https://discord.com/channels/280102180189634562/845299014475972609/1251134009421860875 and ff. https://discord.com/channels/280102180189634562/845299014475972609/1251134026501062718. Overall see the three options for dependencies I point out in the last sentence of the last paragraph.

@mark-at-pieces I don't think and remember that we ever merged something just out of the promise that a PR might land at some point. The key-bindings haven't changed yet. And the code @bishoy-at-pieces shows still claims ctrl+s. Here again, if that has been discussed at Discord, that might be okay. I haven't seen it, and it is very unusual to claim such prominent key-chords. That would be a -1 from me, but maybe others give it a +1 under the circumstances.

braver will typically ask you to add the Telemetry and privacy stuff to the README. Just refer the PR adding OpenAI here in this repository. You're a company so you're even more likely to have interests in user data. That's just how it is. So again, telemetry is opt-in only, and you have to typically add a sentence about it to the README, e.g. that you don't do that. And if you're sending portions of a buffer (user data/code) around, you should be clear about it as well.

[bishoy-at-pieces]

Yes the ctrl+s uses context as you specified

[kaste]

Ah, okay, on Windows but not on Linux/Mac. Is this correct?

[bishoy-at-pieces]

Let me know how can I use the pieces sdks without using the lib since the imports are absolute imports not relative

[kaste]

I just wrote it a) to e) and then the link to discord where I explain different ways and in more detail.

[bishoy-at-pieces]

Ah, okay, on Windows but not on Linux/Mac. Is this correct?

Yes it is updated for mac and Linux in the commit after

[bishoy-at-pieces]

I just wrote it a) to e) and then the link to discord where I explain different ways and in more detail.

Will that work with the packages with absolute path?

[kaste]

There are various options, and you may use a mix of them, and in the end it should work out nicely.

[bishoy-at-pieces]

There are various options, and you may use a mix of them, and in the end it should work out nicely.

Not sure exactly what should I do about handling the sdks dependencies if you can guide me with some docs to add the sdks(absolute path won't work with a lib folder) I will be thankful

[kaste]

I don't have more docs just at my hand beside what I wrote up here and on Discord. I also don't understand what you don't understand as you don't ask anything specific. Just start with "a)" because that seems easy to me. But that doesn't help you of course if you're in the nebulae.

[bishoy-at-pieces]

So basically I need the Pieces OS sdks, and not sure how to add it as a dependency
a) the sdks uses absolute path so I cant use lib method
b) I would like also to add the websocket-client support for python 3.8 in the channel control

[kaste]

I gave an enumeration a) to e). Please start with what can be categorized into "a)". Do the SDK not yet but last. In which categories does the SDK actually fit?

[bishoy-at-pieces]

Not sure what you mean by "category"
Here is the SDKs if you would like to take a look
https://github.com/pieces-app/pieces-os-client-sdk-for-python
I am not sure by adding to esoteric libraries also and vendoring the SDKs won't work

[kaste]

The conversation doesn't seem to be very productive. I have to stop this for now.

[bishoy-at-pieces]

@kaste I vendor the SDKs as you asked me is this ready to go or anything is still missing?

[packagecontrol-bot]

Automated testing result: SUCCESS

Packages modified:
  - Pieces

[braver]

would love to know how our organization can verify ownership of the project rather than ownership given to an individual user.

Using GitHub organizations works well for that.

The replacement of the original repo is fine by the way, the story lines up here.

I just need to find some more time to read this thread an review the package. Please don’t ping me in the mean time, that tends to have the opposite effect 😉

[mark-at-pieces]

Hey all, just wanted to check in an see how are are doing on this pr to package control and see if there is anything that we can do on our side to help out.

Thanks again for all your help!

[kaste]

That gets a LGTM from me, basically from the technial POV. @braver for the rest and the merge.

It is a bit tiring that I ask about about data collection, and then see

IMO that's so weaseling where OpenAI just puts

in the README. But you do you.

Also, you should have pinged us earlier, right after merging the new key-bindings which was on Jun 25, but your last post here was on Jun 18.

[bishoy-at-pieces]

Yes you can use our local models there are local model that don't collect data but no client side data collection anyways thank you

[kaste]

I open 2 (?) trivial issues on your repo.

[braver]

see if there is anything that we can do on our side to help out.

I would say that at this point we're mostly waiting for you to finalize ongoing work based the feedback and otherwise, and tag a new release:

[bishoy-at-pieces]

see if there is anything that we can do on our side to help out.

I would say that at this point we're mostly waiting for you to finalize ongoing work based the feedback and otherwise, and tag a new release:

@braver I just tagged a release
https://github.com/pieces-app/plugin_sublime/releases/tag/1.1.0