Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

guysoft
Copy link

@guysoft guysoft commented Dec 14, 2021

As suggested in #1618 , removing flags that are not required

As suggested in  ValveSoftware#1618 , removing flags that are not required
@okawo80085
Copy link

Way better, but looking at the whole Linux section in the README again, they used an actual make command make -j4 instead of cmake --build . after generating the build files. Why? Hell if i know, but that's not the way cmake builds should be, cmake --build . works on Linux too and in this case would be more appropriate, hell even the -j 4 argument works with the cmake build command.

@okawo80085
Copy link

You don't need to reopen a PR for this change tho xD

Just add a new commit to this one.

@tort-oise
Copy link

it is how it is now B)

@guysoft
Copy link
Author

guysoft commented Dec 14, 2021

I actually like separating a new cmake command to cmake and build, because it lets you see when you got cmake working,a when the actual build fails. Its more useful when troubleshooting

@okawo80085
Copy link

So do i, it does indeed help if the cmake generate and build commands are separated, im just saying that the build command they're using is not ideal.

They have this there right now:

To build type:
make -j4

When it should be this:

To build type:
cmake --build . -j 4

This covers for this one rare case when there is no Makefile on the system, but some other generator is available and was picked by the previous generate command.

Its still a separate command, just a more robust one.

@guysoft
Copy link
Author

guysoft commented Dec 15, 2021

I had an issue with no make file on the system, because I didn't know from what folder to build. That actually happens to me here :)

@okawo80085
Copy link

Oh xD

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.