Skip to content

Conversation

@mahmoodzaker
Copy link

  • Graph Basics
  • All Node types
  • Toolbar and Properties Dockable panels
  • Process Node variant ports
  • Property Browser

@Llcoolsouder
Copy link
Collaborator

No one can reasonably review 300k+ lines of code.
Mostly it looks like you:

  • copied in the source from a new dependency (there are other ways to go about this)
  • added an example app (a visual shader/GPGPU programming app maybe?)
  • Made some relatively small changes to the nodeeditor library
  1. Does my assessment seem accurate?
  2. Is there some smaller piece of this that can be separated from the rest to start with? (maybe my 3rd bullet point?)
  3. I really think the example application added, while pretty cool, is beyond the scope of just being a minimal example with which library consumers can follow along. (I'm guessing this would also remove the need for the new thirdparty deps introduced here.)

@paceholder
Copy link
Owner

paceholder commented Sep 2, 2025

Hi @mahmoodzaker ,

I believe you are building something great but I am afraid we can't merge it to the library in the current form.

  • As @Llcoolsouder has mentioned, this PR is huge and barely explains its purpose
  • I checked your code and it does not compile due to missing dependency files from external/qt-property-browser
  • I do not think we want to pull-in some huge dependencies just for the sake of adding a very large and compilecated demo to the library.

If you are working on some large independent project using QtNodes, do ping us later when it starts to work as expected, we can references your project in README.
Alternatively, re-submit the pr using the currently active pull-request-form that has all the necessary explaination sections.
I decline it now, because I still see random commits coming that makes a lot of noise in my e-mails

Regards
Dimitry

@paceholder paceholder closed this Sep 2, 2025
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.

3 participants