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.

Sign up for GitHub

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

Jump to bottom

README: add recommended release options #221

Closed
wants to merge 2 commits into from
Closed

README: add recommended release options #221

wants to merge 2 commits into from

Conversation

lucasdemarchi
Copy link
Contributor

Related to #220

evelikov
evelikov suggested changes Nov 4, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
build-release.ini Outdated Show resolved Hide resolved
build-release.ini Outdated Show resolved Hide resolved
@evelikov evelikov mentioned this pull request Nov 4, 2024
Closed
@dileks
Copy link
Contributor

dileks commented Nov 5, 2024

Dunno what the default is for strip options - might be explicitly set to true.

What about an ini directory where both ini files are placed?

@lucasdemarchi
README: Let autotools follow the same steps as meson
8b2c166 
For meson it's configure & build & install in the same snippet.
Do the same for autotools.

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
evelikov
evelikov approved these changes Nov 7, 2024
View reviewed changes
Copy link
Collaborator

@evelikov evelikov left a comment

Choose a reason for hiding this comment

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

With the compression/openssl defaulting to "no", the "automatically searches" comment is quite misleading IMHO.

So yes please.

@lucasdemarchi
README: Add release/distro/end-user recommendation
6549b6d 
This may be useful for anyone trying a non-debug build and serve as
baseline for distros. Even for developers, when benchmarking kmod, it's
better to run a release-oriented kmod rather than the slow one due to
all debug/sanitizers.

By keeping the -D in the command line rather than in a ini file,
we also guarantee meson shows it in the summary, regardless of
mesonbuild/meson#13865.

Closes: #220
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
@lucasdemarchi
Copy link
Contributor Author

What about an ini directory where both ini files are placed?

we decided to keep just one ini file. Defaults are a "sane" set and --buildtype is recommended to be kept in the command line, which also means it will show up in the summary as you requested.

@lucasdemarchi
Copy link
Contributor Author

With the compression/openssl defaulting to "no", the "automatically searches" comment is quite misleading IMHO.

sorry, what comment are we talking about?

@evelikov
Copy link
Collaborator

evelikov commented Nov 7, 2024

The first commit removes the "... automatically searches for all required components..." which tripped me a few times and again just now. Aka misread "required" as "optional", since the former is a given for any (maintained) project.

Either way, the updated PR looks good 👍

@lucasdemarchi
Copy link
Contributor Author

Ah, ok. Thanks.

lucasdemarchi added a commit that referenced this pull request Nov 7, 2024
@lucasdemarchi
README: Let autotools follow the same steps as meson
18c91d3 
For meson it's configure & build & install in the same snippet.
Do the same for autotools.

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Link: #221
lucasdemarchi added a commit that referenced this pull request Nov 7, 2024
@lucasdemarchi
README: Add release/distro/end-user recommendation
250330e 
This may be useful for anyone trying a non-debug build and serve as
baseline for distros. Even for developers, when benchmarking kmod, it's
better to run a release-oriented kmod rather than the slow one due to
all debug/sanitizers.

By keeping the -D in the command line rather than in a ini file,
we also guarantee meson shows it in the summary, regardless of
mesonbuild/meson#13865.

Closes: #220
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Link: #221
@lucasdemarchi
Copy link
Contributor Author

Applied, thanks for reviewing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evelikov evelikov evelikov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lucasdemarchi @dileks @evelikov