fabric icon indicating copy to clipboard operation
fabric copied to clipboard

refactor: use slices.Contains to simplify code

Open keeghcet opened this issue 4 months ago • 8 comments

Type of change

  • Improvement (improvement to code, performance, etc)

Description

There is a new function added in the go1.21 standard library, which can make the code more concise and easy to read.

Additional details

Related issues

keeghcet avatar Sep 08 '25 09:09 keeghcet

Technically, you did the right thing, but in my opinion, it can be improved. Try to do better.

Thanks for your review! Modified!

keeghcet avatar Sep 08 '25 10:09 keeghcet

Technically, you did the right thing, but in my opinion, it can be improved. Try to do better.

Thanks for your review! Modified!

please achieve a green pipline

pfi79 avatar Sep 08 '25 12:09 pfi79

Technically, you did the right thing, but in my opinion, it can be improved. Try to do better.

Thanks for your review! Modified!

please achieve a green pipline

Thanks. Modified.

keeghcet avatar Sep 09 '25 08:09 keeghcet

@pfi79 I have fix the mock test file, Please approve the CI and review again when you have time.

image

keeghcet avatar Sep 10 '25 08:09 keeghcet

@keeghcet Look at the tests. It seems that some kind of test was touched. The test dropped twice in one place.

pfi79 avatar Sep 10 '25 12:09 pfi79

@keeghcet Look at the tests. It seems that some kind of test was touched. The test dropped twice in one place.

It seems that the assertion for the error should be nil because the error return value has been removed.

--- FAIL: TestSnapshotImporterErrorPropagation (0.09s)
    --- FAIL: TestSnapshotImporterErrorPropagation/error-when-membershipProvider-returns-error (0.01s)
        snapshot_data_importer_test.go:512: 
            	Error Trace:	/home/runner/work/fabric/fabric/core/ledger/pvtdatastorage/snapshot_data_importer_test.go:512
            	Error:      	An error is expected but got nil.
            	Test:       	TestSnapshotImporterErrorPropagation/error-when-membershipProvider-returns-error

Modified.

keeghcet avatar Sep 10 '25 13:09 keeghcet

@keeghcet Look at the tests. It seems that some kind of test was touched. The test dropped twice in one place.

It seems that the assertion for the error should be nil because the error return value has been removed.

--- FAIL: TestSnapshotImporterErrorPropagation (0.09s)
    --- FAIL: TestSnapshotImporterErrorPropagation/error-when-membershipProvider-returns-error (0.01s)
        snapshot_data_importer_test.go:512: 
            	Error Trace:	/home/runner/work/fabric/fabric/core/ledger/pvtdatastorage/snapshot_data_importer_test.go:512
            	Error:      	An error is expected but got nil.
            	Test:       	TestSnapshotImporterErrorPropagation/error-when-membershipProvider-returns-error

Modified.

Take a look at this test error-when-membershipProvider-returns-error. He doesn't match the description right now. We need to do something about it. It may need to be deleted.

pfi79 avatar Sep 10 '25 14:09 pfi79

@keeghcet Look at the tests. It seems that some kind of test was touched. The test dropped twice in one place.

It seems that the assertion for the error should be nil because the error return value has been removed.

--- FAIL: TestSnapshotImporterErrorPropagation (0.09s)
    --- FAIL: TestSnapshotImporterErrorPropagation/error-when-membershipProvider-returns-error (0.01s)
        snapshot_data_importer_test.go:512: 
            	Error Trace:	/home/runner/work/fabric/fabric/core/ledger/pvtdatastorage/snapshot_data_importer_test.go:512
            	Error:      	An error is expected but got nil.
            	Test:       	TestSnapshotImporterErrorPropagation/error-when-membershipProvider-returns-error

Modified.

Take a look at this test error-when-membershipProvider-returns-error. He doesn't match the description right now. We need to do something about it. It may need to be deleted.

Thanks for your help. Removed!

keeghcet avatar Sep 10 '25 15:09 keeghcet