Skip to content

Conversation

@wthks
Copy link

@wthks wthks commented Dec 29, 2021

electron app does not show anymore after update. it only can be shown in localhost:8001

@danatcofo
Copy link
Contributor

What's the purpose of this PR? What does it accomplish? You have a lot of file changes here with no explanation.

@wthks
Copy link
Author

wthks commented Dec 30, 2021

What's the purpose of this PR? What does it accomplish? You have a lot of file changes here with no explanation.

its probally a continue of the project. the current version that works for me is this 16.0.2.23861.
I actually also figured out that this pr starts somehow faster than the current version.

@danatcofo may I ask if there is a way to make NeutralinoJS the same way as electron?
neutralino compared to electron is way better.
https://github.com/Elanis/web-to-desktop-framework-comparison

@danatcofo
Copy link
Contributor

danatcofo commented Dec 30, 2021

I'll take a look at the pr later but I'd still like a bullet list or something of what was done.

As for applying this same concept to another project, all this does is provide an interface for the .net side of things to call standard electron stuff. It's middleware at the heart of it. You'd need to write something with the same thought and goal for neutrino. All the heavy lifting is done by other projects here.

Sorry typing this from a phone out in the middle of the woods lol.

@danatcofo
Copy link
Contributor

danatcofo commented Dec 31, 2021

So I took a gander at everything you have here and I feel like there is way to much here to just bring into the main repo wholesale. Much of it looks cosmetic and a lot of it looks like stuff we would want but its months of work all compiled together and includes your own build scripts. This level of change requires a lot of testing that I would be uncomfortable with given the scope of your changes.

There are some things I love in here like the OS targeted features/methods and the use of OS targeted packages and implementations based on support. I think those things are wonderful. Even the cosmetic stuff to simplify the access of the socket stuff is nice but at the same time. I think that there has to be much more provided here to the PR in terms of documentation than what you have provided. Honestly this is kind of just thrown over the wall and as an Open Source project I don't think it meets the requirements for a PR given the scope of the change.

Ideally this would be a number of PR's separated out and targeted for consumption of THIS repo rather than one big one that is targeted towards you own usage. Or maybe if the changes require THAT much change we have @GregorBiswanger create a major version branch to increment it there and thus be able to migrate other changes.

And I totally understand having your own usage branch, I use my own branch for use with my application but I'm not going to take my changes wholesale here as they contain changes that don't take into consideration the current state of this repo.

I just noticed you also moved to .net 6 and didn't fix the appveyor build. look at #636 for how to get appveyor working with .net 6

@wthks wthks closed this Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants