Skip to content

Conversation

keeghcet
Copy link

@keeghcet keeghcet commented Sep 8, 2025

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 keeghcet requested a review from a team as a code owner September 8, 2025 09:26
Copy link
Contributor

@pfi79 pfi79 left a comment

Choose a reason for hiding this comment

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

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

@keeghcet
Copy link
Author

keeghcet commented Sep 8, 2025

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

Thanks for your review! Modified!

@pfi79
Copy link
Contributor

pfi79 commented Sep 8, 2025

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

@keeghcet
Copy link
Author

keeghcet commented Sep 9, 2025

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
Copy link
Author

keeghcet commented Sep 10, 2025

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

image

@keeghcet keeghcet force-pushed the main branch 2 times, most recently from ad8bdea to d2743de Compare September 10, 2025 10:36
pfi79
pfi79 previously approved these changes Sep 10, 2025
@pfi79
Copy link
Contributor

pfi79 commented Sep 10, 2025

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

@keeghcet
Copy link
Author

@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.

@pfi79
Copy link
Contributor

pfi79 commented Sep 10, 2025

@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.

@keeghcet
Copy link
Author

@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!

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.

2 participants