fstl-app / fstl

A fast STL file viewer
449 stars 105 forks source link

Remove Legacy Qt Build Option #62

Closed jackie-scholl closed 3 years ago

jackie-scholl commented 3 years ago

Master failed to build for me on NixOS; I tracked the issue down to axis.cpp not being built in the versions that had it, because it wasn't tracked in fstl.pro. I'm not entirely sure how it works to be honest, but at least for me, this patch gets it building.

mboerwinkle commented 3 years ago

Cool. Should we also take this opportunity to add in vector.h (used by loader.cpp)? My intuition tells me yes, but I know almost nothing about qt .pro files. I just noticed it was missing too, and wanted to say something. Thanks, Martin

DeveloperPaul123 commented 3 years ago

Thanks for bringing this up. We haven't really been keeping the Qt creator build updated since CMake support was added. I can try to take a a look as well.

jackie-scholl commented 3 years ago

I can also experiment with moving nixpkgs to use a CMake-based build, if you think that's the better choice long-term

EDIT: I was able to get a CMake-based build working in nix, if you decide that CMake is the preferred build system then I'll submit a PR to nixpkgs. I do think it's worth deciding to either put the two build systems on equal footing and test them both on every PR, or deprecate one of them.

DeveloperPaul123 commented 3 years ago

Personally I think the CMake-based build is the best bet moving forward. I haven't used the Qt based build in some time and not sure if it's even needed anymore.

@mkeeter Any reason to keep the .pro file around anymore since we have a CMake based build now?

mkeeter commented 3 years ago

Nope – CMake is definitely the wave of the future for Qt-based apps, so I'm fine removing the .pro file.

DeveloperPaul123 commented 3 years ago

@raptortech-js If you update the PR to remove the .pro file, I'll happily merge it in.

jackie-scholl commented 3 years ago

@DeveloperPaul123 Done, thanks!

DeveloperPaul123 commented 3 years ago

Thanks @raptortech-js !