Skip to content

Fix eventhubs manifest - #4812

Merged
Anton Kolesnyk (antkmsft) merged 4 commits into
Azure:mainfrom
antkmsft:messaging-eventhubs
Jul 20, 2023
Merged

Fix eventhubs manifest #4812
Anton Kolesnyk (antkmsft) merged 4 commits into
Azure:mainfrom
antkmsft:messaging-eventhubs

Conversation

@antkmsft

@antkmsft Anton Kolesnyk (antkmsft) commented Jul 20, 2023

Copy link
Copy Markdown
Member

vcpkg manifest has it under /messaging/:

SOURCE_PATH "${SOURCE_PATH}/sdk/messaging/azure-messaging-eventhubs/"

(Currently, vcpkg validation run microsoft/vcpkg#17119 fails because this path does not exist)

We could fix either, but the include file path is messaging/eventhubs, plus it is more consistent with storage and keyvault.

Go SDK has it under messaging: https://github.com/Azure/azure-sdk-for-go/tree/main/sdk/messaging/azeventhubs

@LarryOsterman

Larry Osterman (LarryOsterman) commented Jul 20, 2023

Copy link
Copy Markdown
Member

The Go SDK is correct according to the rules of go.

The C++ SDK follows the patterns of all the other languages and there is no overarching reason to align the C++ repo with the Go repo instead of the other languages.

Putting the EventHubs service in the EventHubs directory was quite intentional and should not be changed without a VERY strong reason, and "compatibility with Go" is not a strong reason, IMHO.

And at a minimum, moving the ci.yml file from the EventHubs directory to messaging\eventhubs messes up the name of the pipeline created by the create-pipelines pipeline (instead of the pipeline being cpp - eventhubs - ci, it turns into cpp - messaging-eventhubs - ci, which is different from the other pipelines.

The vcpkg manifest should be corrected to reflect the source tree layout instead of the source tree layout being changed to reflect the vcpkg manifest.

@antkmsft Anton Kolesnyk (antkmsft) changed the title Move eventhubs under messaging Fix eventhubs manifest Jul 20, 2023
@antkmsft

Copy link
Copy Markdown
Member Author

Larry Osterman (@LarryOsterman), I inverted the change, please check it out now.

@antkmsft
Anton Kolesnyk (antkmsft) merged commit 7d52c01 into Azure:main Jul 20, 2023
@antkmsft
Anton Kolesnyk (antkmsft) deleted the messaging-eventhubs branch July 20, 2023 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

EngSys This issue is impacting the engineering system. Event Hubs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants