[osalyk]

This change is 

[janekmi]

@janekmi reviewed 65 files and all commit messages, and made 13 comments.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on grom72 and osalyk).

---

-- commits line 4 at r1:
"obsolete bencharks" sounds like there are benchamrks which are obsolete but there are also benchamrks which we left out. Which is not the case.

Suggestion:

- f3f5b33: common: PMDK-656: remove benchmarks

All benchmarks are deemed obsolete and a burden to maintain due to the use of C++ instead of C as the rest of this repository.

  Signed-off-by: Oksana Salyk <oksana.salyk@hpe.com>

---

CONTRIBUTING.md line 64 at r1 (raw file):

This will check all C/C++ files in the tree for style issues. If you want to

Suggestion:

This will check all C and Python files in the tree for style issues.

---

src/common.inc line 222 at r1 (raw file):

ifeq ($(CSTYLEON),1)
define check-cstyle
	@$(STYLE_CHECK) check $1 && if [ "$2" != "" ]; then mkdir -p `dirname $2` && touch $2; fi

Suggestion:

@$(STYLE_CHECK) $1

---

src/common.inc line 226 at r1 (raw file):

else ifeq ($(CSTYLEON),2)
define check-cstyle
	@$(STYLE_CHECK) check $1 && if [ "$2" != "" ]; then mkdir -p `dirname $2` && touch $2; fi || true

Suggestion:

@$(STYLE_CHECK) $1

---

src/Makefile line 175 at r1 (raw file):

cstyle:
	$(STYLE_CHECK) check $(HEADERS)

Suggestion:

$(STYLE_CHECK) $(HEADERS)

---

src/Makefile line 179 at r1 (raw file):

format:
	$(STYLE_CHECK) format $(HEADERS)

---

utils/style_check.sh line 21 at r1 (raw file):

function usage() {
	echo "$0 <check|format> [C/C++ files]"
}

---

utils/style_check.sh line 53 at r1 (raw file):

		*.[ch]pp)
			CLANG_ARGS+="${ARGS[$i]} "
			;;

---

utils/style_check.sh line 77 at r1 (raw file):

    usage
    exit 1
fi

Suggestion:

run_cstyle ${CSTYLE_ARGS}
run_flake8 ${FLAKE8_ARGS}

---

utils/docker/images/Dockerfile.opensuse-leap-15 line 93 at r1 (raw file):

	$BASE_DEPS \
	$VALGRIND_DEPS \
	$BENCH_DEPS \

---

utils/docker/images/Dockerfile.rockylinux-8 line 92 at r1 (raw file):

	$BASE_DEPS \
	$VALGRIND_DEPS \
	$BENCH_DEPS \

---

utils/docker/images/Dockerfile.rockylinux-9 line 90 at r1 (raw file):

	$BASE_DEPS \
	$VALGRIND_DEPS \
	$BENCH_DEPS \

---

utils/docker/images/Dockerfile.ubuntu-22.04 line 88 at r1 (raw file):

	$BASE_DEPS \
	$VALGRIND_DEPS \
	$BENCH_DEPS \

[osalyk]

@osalyk partially reviewed 65 files and made 12 comments.
Reviewable status: 57 of 65 files reviewed, 13 unresolved discussions (waiting on grom72 and janekmi).

---

CONTRIBUTING.md line 64 at r1 (raw file):

This will check all C/C++ files in the tree for style issues. If you want to

Done.

---

src/common.inc line 222 at r1 (raw file):

ifeq ($(CSTYLEON),1)
define check-cstyle
	@$(STYLE_CHECK) check $1 && if [ "$2" != "" ]; then mkdir -p `dirname $2` && touch $2; fi

Done.

---

src/common.inc line 226 at r1 (raw file):

else ifeq ($(CSTYLEON),2)
define check-cstyle
	@$(STYLE_CHECK) check $1 && if [ "$2" != "" ]; then mkdir -p `dirname $2` && touch $2; fi || true

Done.

---

src/Makefile line 175 at r1 (raw file):

cstyle:
	$(STYLE_CHECK) check $(HEADERS)

Done.

---

src/Makefile line 179 at r1 (raw file):

format:
	$(STYLE_CHECK) format $(HEADERS)

Done.

---

utils/style_check.sh line 21 at r1 (raw file):

function usage() {
	echo "$0 <check|format> [C/C++ files]"
}

Done.

---

utils/style_check.sh line 53 at r1 (raw file):

		*.[ch]pp)
			CLANG_ARGS+="${ARGS[$i]} "
			;;

Done.

---

utils/style_check.sh line 77 at r1 (raw file):

    usage
    exit 1
fi

Done.

---

utils/docker/images/Dockerfile.opensuse-leap-15 line 93 at r1 (raw file):

	$BASE_DEPS \
	$VALGRIND_DEPS \
	$BENCH_DEPS \

Done.

---

utils/docker/images/Dockerfile.rockylinux-8 line 92 at r1 (raw file):

	$BASE_DEPS \
	$VALGRIND_DEPS \
	$BENCH_DEPS \

Done.

---

utils/docker/images/Dockerfile.rockylinux-9 line 90 at r1 (raw file):

	$BASE_DEPS \
	$VALGRIND_DEPS \
	$BENCH_DEPS \

Done.

---

utils/docker/images/Dockerfile.ubuntu-22.04 line 88 at r1 (raw file):

	$BASE_DEPS \
	$VALGRIND_DEPS \
	$BENCH_DEPS \

Done.

[osalyk]

@osalyk made 1 comment.
Reviewable status: 57 of 65 files reviewed, 13 unresolved discussions (waiting on grom72 and janekmi).

---

-- commits line 4 at r1:

Previously, janekmi (Jan Michalski) wrote…

"obsolete bencharks" sounds like there are benchamrks which are obsolete but there are also benchamrks which we left out. Which is not the case.

Done.

[janekmi]

I see a number of format targets mentioned in:

• src/test/Makefile,
• src/tools/Makefile

They should probably be removed.

@janekmi reviewed 8 files and all commit messages, made 5 comments, and resolved 13 discussions.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on grom72 and osalyk).

---

src/common.inc line 218 at r2 (raw file):

cstyle: cstyle-check

format: cstyle-format

---

src/Makefile line 89 at r2 (raw file):

clobber: $(CLOBBER_TARGETS:=-clobber)
cstyle: $(CSTYLE_TARGETS:=-cstyle)
format: $(CSTYLE_TARGETS:=-format)

---

src/Makefile line 119 at r2 (raw file):

$(eval $(call sub-target,$(CLOBBER_TARGETS),clobber,y))
$(eval $(call sub-target,$(CSTYLE_TARGETS),cstyle,n))
$(eval $(call sub-target,$(CSTYLE_TARGETS),format,n))

---

src/Makefile line 189 at r2 (raw file):

clobber: clean-here

.PHONY: all install uninstall clean clobber cstyle format test check pcheck\

Suggestion:

.PHONY: all install uninstall clean clobber cstyle test check pcheck\

[osalyk]

Done.

@osalyk made 5 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on grom72 and janekmi).

---

src/common.inc line 218 at r2 (raw file):

cstyle: cstyle-check

format: cstyle-format

Done.

---

src/Makefile line 89 at r2 (raw file):

clobber: $(CLOBBER_TARGETS:=-clobber)
cstyle: $(CSTYLE_TARGETS:=-cstyle)
format: $(CSTYLE_TARGETS:=-format)

Done.

---

src/Makefile line 119 at r2 (raw file):

$(eval $(call sub-target,$(CLOBBER_TARGETS),clobber,y))
$(eval $(call sub-target,$(CSTYLE_TARGETS),cstyle,n))
$(eval $(call sub-target,$(CSTYLE_TARGETS),format,n))

Done.

---

src/Makefile line 189 at r2 (raw file):

clobber: clean-here

.PHONY: all install uninstall clean clobber cstyle format test check pcheck\

Done.

[janekmi]

@janekmi reviewed 8 files and all commit messages, made 4 comments, and resolved 4 discussions.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on grom72 and osalyk).

---

src/test/Makefile line 306 at r3 (raw file):

test    : TARGET = test
cstyle  : TARGET = cstyle
format  : TARGET = format

---

src/test/Makefile line 317 at r3 (raw file):

FILE_MAX_DAX_DEVICES=$(TOP)/src/test/tools/anonymous_mmap/max_dax_devices

all test format sparse: $(TESTS_BUILD) envconfig.sh envconfig.py

Suggestion:

all test sparse: $(TESTS_BUILD) envconfig.sh envconfig.py

---

src/tools/Makefile line 22 at r3 (raw file):

clobber: TARGET = clobber
cstyle : TARGET = cstyle
format : TARGET = format

---

src/tools/Makefile line 27 at r3 (raw file):

sparse: TARGET = sparse

all clean clobber cstyle install uninstall check format test sparse: $(TARGETS)

Suggestion:

all clean clobber cstyle install uninstall check test sparse: $(TARGETS)

[osalyk]

@osalyk made 4 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on grom72 and janekmi).

---

src/test/Makefile line 306 at r3 (raw file):

test    : TARGET = test
cstyle  : TARGET = cstyle
format  : TARGET = format

Done.

---

src/test/Makefile line 317 at r3 (raw file):

FILE_MAX_DAX_DEVICES=$(TOP)/src/test/tools/anonymous_mmap/max_dax_devices

all test format sparse: $(TESTS_BUILD) envconfig.sh envconfig.py

Done.

---

src/tools/Makefile line 22 at r3 (raw file):

clobber: TARGET = clobber
cstyle : TARGET = cstyle
format : TARGET = format

Done.

---

src/tools/Makefile line 27 at r3 (raw file):

sparse: TARGET = sparse

all clean clobber cstyle install uninstall check format test sparse: $(TARGETS)

Done.

[janekmi]

@janekmi reviewed 2 files and all commit messages, made 1 comment, and resolved 4 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on grom72).