Skip to content

Fix eventhubs manifest #4812

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

Fix eventhubs manifest #4812
antkmsft merged 4 commits into
Azure:mainfrom
antkmsft:messaging-eventhubs

Conversation

@antkmsft

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

@antkmsft antkmsft added EngSys This issue is impacting the engineering system. Event Hubs labels Jul 20, 2023
@antkmsft antkmsft self-assigned this Jul 20, 2023
@LarryOsterman

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 antkmsft changed the title Move eventhubs under messaging Fix eventhubs manifest Jul 20, 2023
@antkmsft

Copy link
Copy Markdown
Member Author

@LarryOsterman, I inverted the change, please check it out now.

@antkmsft antkmsft merged commit 7d52c01 into Azure:main Jul 20, 2023
@antkmsft 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