Skip to content
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

Remove Visual Studio solution, improve new-contributor experience #5626

Merged
merged 5 commits into from
Sep 14, 2023

Conversation

Web-eWorks
Copy link
Member

This PR removes the old Travis and Appveyor CI files (given that we've used neither in +6 months with no plans to return), and removes the 'manual' Visual Studio solution build method from both our CI matrix and the repo.

At this point @fluffyfreak is the last developer using those files, and with a much nicer initial setup for CMake + "using CMake by default" there's not much point in keeping them around.

This PR also includes a "new contributor experience" improvement for VSCode (specifically on linux, though very likely to work on Windows as well) which allows a contributor to have a working automated build config within a minute of downloading the repo, by copying a default VSCode configuration with pre-set defaults for Lua and our build tooling.

CC @pcercuei; in my eyes this PR handles at least part of what was wanted in #5209. I'd be happy to see that PR re-tooled to extend the default .code-workspace provided in this PR; the main sticking point with the prior solution was the inability for users to add their own launch or build configurations if we checked those files into source control.

Additionally, this PR contains an experimental change to fully build our release archive with CMake's install() rules, reducing the number of "moving parts" in our packaging flow.

@JonBooth78 you mentioned on IRC being willing to give VSCode a whirl 😄 - I'd appreciate it if you'd give the VSCode + CMake section in COMPILING.txt a go on Windows as well.

- Travis has been fully deprecated and nonfunctional for a long time
- Appveyor was superceeded by Github Actions and is no longer active
- The manual visual studio solution is no longer maintained and is being
  replaced by the CMake build process for all developers
- The manually-maintained Visual Studio solution is deprecated in favor of CMake
@JonBooth78
Copy link
Contributor

I had to jump through some hoops to get VSCode to work on windows, but I think they're VSCode hoops and not yours.

For starters, I needed to install CMake and make it available on my path.
And then ninja (which as it just comes as an exe, I copied into the C:\Program Files\CMake\bin folder as I'm pretty lazy).

Those two are steps we'd want to add.

Then as I used VSCode with the VS2019 toolchain I followed their instructions on how to do this had to open a "x64 Native Command Prompt for VS 2019", and then from within the root of my pioneer folder, I ran "c:\Program Files\Microsoft VS Code\bin\code" pioneer.code-workspace"

However, then it all worked as expected 😂 Just using Visual Studio 2019 was much easier.

If I find a bit of time later, I might try it with the g++ workflow from within a MSYS2 environment or similar, that might make installing the dependencies and such easier - I'll have to go and read up on that though.

@pcercuei
Copy link
Contributor

I had to jump through some hoops to get VSCode to work on windows, but I think they're VSCode hoops and not yours.

For starters, I needed to install CMake and make it available on my path. And then ninja (which as it just comes as an exe, I copied into the C:\Program Files\CMake\bin folder as I'm pretty lazy).

Last time I used VSCode (on Windows) it had built-in support for CMake, I don't remember having to install CMake manually.

As for ninja - why do you need it at all? VSCode will generate its own .project files from the CMake script and use these for the build, I don't see at which point you need ninja.

@Web-eWorks
Copy link
Member Author

Building our Github release via CMake instead of a custom script seem to work very well; I've applied the change to all our linux CI builds.

@pcercuei if you could review the changes to CMakeLists.txt and verify it will work correctly with the Flatpak packaging configuration I'd appreciate it. IIRC you were the one to last modify/contribute our Flatpak config, and I do not have much experience with that aspect of our releases.

If everything checks out, I'll be merging this PR soon.

Copy link
Contributor

@fluffyfreak fluffyfreak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aww man, first thing I ever did on Pioneer was fix up the VS projects and get things building again :( still time moves on. I cannot comment on the CMake configs, it looks like ti make sense and I guess it works. So long as it produces functioning builds then it's all good.

@Web-eWorks
Copy link
Member Author

Aww man, first thing I ever did on Pioneer was fix up the VS projects and get things building again :( still time moves on.

And thank you for maintaining them for the last decade (or more)! Hopefully this way you can spend more of your little bit of free time testing and playing Pioneer, and less mucking around with build issues. 😄

@Web-eWorks Web-eWorks merged commit 1957b8d into pioneerspacesim:master Sep 14, 2023
4 checks passed
@Web-eWorks Web-eWorks deleted the remove-vs-solution branch October 8, 2023 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants