-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
update to gnome 46 #264
update to gnome 46 #264
Conversation
I would suggest, that we keep backwards compatibility with gnome 45, since we did that before, and it's best practice, it is detectable, if |
AFAIK in gnome 45 add_actor was already deprecated and add_child should work as well. |
Yes, other work in the wild confirms 45 and 46 both being fine with this upgrade fflewddur/tophat#122 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also tested locally, works on gnome 45 and 46 👍🏼
Another note from me, since it's not mentioned, EGO Reviewers said, that we need to use fewer third party libraries (see e.g. #222 (comment)), so this PR is ready to merged imo @oae . It's unrelated to this PR, but i'll say it anywhere, do we now what the exact reasoning behind that statement from EGO is, and what are we supposed to do in their view, since we definitely need these third party dependencies (e.g. highlightjs)? |
I don't know the exact reason, but I think third-party libraries in Pano make the reviews very difficult for them. So, they don't want me to include them anymore. I didn't have time to think of a solution. If we can load these libraries dynamically somehow, we can ask the user to accept them during the first start and download them from Github. But at this point, this extension becomes a glorified installer. Maybe it is better to not update on EGO anymore. |
@underdoeg thank you for the pr <3 |
Or we create a deb / rpm / whatever (depending on the distro) for the needed files, and you need to install those, but thats equally bad... Regarding #222 (comment) I would ask you @jrahmatzadeh (the author of that comment ) I read the guidelines And I found Which to me sounds like we are in that area, since we need those external scripts /dependencies for pano to work... There is this section: Is there an example, how one can do that, without forcing the user to install node, npm and the dependencies we need, and than we need to use some version of these dependencies, we don't know, it could be, that we are compatible with the version, the user installed, but it also could not be. It also could be, that the user needs another version for his local development or some similar reasons. So its 100% better imo to include prepackaged dependecies, that work with the extension than doing it this way. We speak about 37 external files atm, so it's not a few 100 / 1000 external scripts, we could bake those dependencies in the extension or maybe reduce it by 1 or 3 files, but the I am an user, that can do the steps like installing global fixed version packages from npm, but not every user can do that and shouldn't have to. On EGO it says @jrahmatzadeh You said, you discussed it in the matrix channel, is there a chance we can find a solution for this together, I get your point of using dependencies / external scripts, but there has to be a solution? (Sorry if it came over harsh, but I used gpaste and other similar extensions than this one, and this is by far the best one ❤️ , so I want it to be available on EGO and accessible to users) |
The extension description already mentions the libgda dependency. You can add other dependencies in the description too. I believe loading many third party libraries in GNOME Shell process isn't good either. Btw, have you considered moving this project to a gjs app rather than an extension? That way, you will be able to do whatever you wanna do in your app outside the shell process and the GNOME Shell extension can only implement the indicator part. It can be available on other desktops and even other OSs. For example, kando is a project based on Fly-Pie extension. |
Long term it is possible to rewrite it, to be an app, but short term it's not easy, we understand, that using too many third party dependencies is not the best option, but we rely on some for the extension to work, we will discuss this and find a solution, that either satisfies the rules of EGO or we distribute it in some other way. It is certainly possible to make the user install those and explain why those dependencies are needed to work, but that requires some thought and intuitive UI or package, using global npm packages is the worst solution possible. Just one question, before we will discuss the rest internally and maybe reach out to you at the end. (Btw it's not the best for both of us, to just ping you @jrahmatzadeh all the time, is there way we could communicate better, without always "bothering" you personally?) The question is, how much is too many? We currently use those files as part of the extension
Some of those are essential for the extension and their name already say, what they are doing. By "just" excluding Thank you in advance for your patience and your answers. |
I'm almost always on GNOME Extensions Matrix Channel. And yeah, removing code highlighter can be sufficient to pass the review (read the discussion on Matrix). |
I am ok with this. The code type in the Pano is not accurate anyway. We can remove it. |
It needs some overhaul to be really helpful (e.g. displaying the language it is in) than it would be helpful (at least for me) So I would say, we disable it for now, but overhaul it, so that an user can install some things manually, to enable it back again. I would do the work, if you don't want to @oae |
Thank you <3 |
I've read the matrix channel, and sorry for asking some duplicate questions, but I didn't read it before 😓 |
I see this is complicated, but just to add my few cents: so far I really liked that Pano highlighted my code syntax. It made it all feel really polished, and made quicly looking through my pastes easy ("oh, some colorful text, must be my code") But if it must go... then fine :( |
I also like it and find it useful, so we are going to include it again, but in a different way, and opt in by the user (by e.g. installing a package) |
Pull Request Template
Description
Simple update for gnome 46. Seems to work but was not thoroughly tested.
Fixes #259
Type of change
Checklist