Skip to content

Conversation

@dkropachev
Copy link
Contributor

It is frustrating, we should not do it.

Fixes: #401

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have implemented Rust unit tests for the features/changes introduced.
  • I have enabled appropriate tests in Makefile in {SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER.
  • I added appropriate Fixes: annotations to PR description.

It is frustrating, we should not do it.
@dkropachev dkropachev requested a review from Lorak-mmk December 15, 2025 16:17
@rm -rf /tmp/download-cassandra.ccm

run-test-integration-scylla: prepare-integration-test download-ccm-scylla-image
run-test-integration-scylla: .prepare-environment-update-aio-max-nr prepare-integration-test
Copy link
Contributor

Choose a reason for hiding this comment

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

Do I understand correctly that it will still call prepare-integration-test, which is something that prepares the environment? What it does:

  • prepare-environment-install-libc which calls dpkg
  • update-apt-cache-if-needed which calls apt-get
  • install-valgrind-if-missing which calls apt

It is fine to check if required tools are installed, and error out if not, but lets not implicitly install them. Installing in separate target is fine. Can you remove prepare-integration-test from user-facing targets (such as run-test-integration-scylla)?

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.

Makefile is frustrating to use by developers

3 participants