Skip to content

Conversation

pfi79
Copy link
Contributor

@pfi79 pfi79 commented Sep 5, 2025

No description provided.

@pfi79 pfi79 requested a review from a team as a code owner September 5, 2025 11:41
}

// swagger:operation GET /v1/participation/channels/{channelID} channels listChannel
// swagger:operation GET /participation/v1/channels/{channelID} channels listChannel
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you change it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the code, everywhere orderer and osnadmin we use "/participation/v1/". And only in the swagger file the path is reversed "/v1/participation/". This option is not working. This means that no one has used the swagger file yet.
Of the two options, I chose to leave the one that is exactly being used, respectively, and fixed the other one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The description of // swagger:operation only helps the program generate the file swagger.json doesn't affect anything else.
The actual path pattern is defined below, and usually uses the constant URLBaseV1

Copy link
Member

Choose a reason for hiding this comment

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

Putting the version number ahead is preferable. When you update to a newer version, it is easy to distinguish from the very beginning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alas, this is inherited. I know it's better this way, but then we'll break backward compatibility. Because so far everything has worked when the version was in second place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now there is a lot of code that already uses the version in the 2nd place.
For example: https://github.com/hyperledger/fabric-admin-sdk/blob/main/internal/osnadmin/join.go#L19

@yeasy
Copy link
Member

yeasy commented Sep 11, 2025

Why is the description empty?
It would be helpful to add some commit message at least.

@pfi79 pfi79 force-pushed the add-osn-get-block2 branch from c8bdcba to 53c1b05 Compare September 11, 2025 19:46
@C0rWin C0rWin merged commit 797abb3 into hyperledger:main Sep 15, 2025
15 checks passed
@pfi79 pfi79 deleted the add-osn-get-block2 branch September 15, 2025 10:41
@yacovm
Copy link
Contributor

yacovm commented Sep 15, 2025

@C0rWin shall we cut a release on the weekend?

@pfi79
Copy link
Contributor Author

pfi79 commented Sep 15, 2025

shall we cut a release on the weekend?

Please wait for 2 pr. I need to finish the theme.
Add new functionality and integration tests.

@yacovm
Copy link
Contributor

yacovm commented Sep 15, 2025

shall we cut a release on the weekend?

Please wait for 2 pr. I need to finish the theme. Add new functionality and integration tests.

Sure

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.

4 participants