Skip to content
This repository has been archived by the owner on Dec 8, 2023. It is now read-only.

Allow creating multiple connections and select which one to use #27

Merged
merged 4 commits into from
Aug 2, 2023

Conversation

swantzter
Copy link
Contributor

fixes #24

@CLAassistant
Copy link

CLAassistant commented May 16, 2023

CLA assistant check
All committers have signed the CLA.

@swantzter
Copy link
Contributor Author

swantzter commented May 16, 2023

I tried to keep this non-breaking which in some cases resulted in an added positional argument that takes an options struct, but relies on existing options structs if they exist. But I'm open to other alternatives, even going to options arguments across the board and thus breaking the API

@javaducky javaducky self-requested a review July 18, 2023 17:08
Copy link
Contributor

@javaducky javaducky left a comment

Choose a reason for hiding this comment

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

Please rebase as this has merge conflicts given merge of #26 .

@swantzter
Copy link
Contributor Author

@javaducky sorry for the delay, I was enjoying a vacation away from a proper computer :) think it should be sorted now

@javaducky
Copy link
Contributor

@swantzter thank you! Looks like there are some linter issues related to missing documentation for the *.GetConn functions.

@swantzter
Copy link
Contributor Author

@javaducky yeah I was looking at them, but I can't see how what I've done differs from how the rest of the library code does it, do you have any pointers for how to align it?

Copy link
Contributor

@javaducky javaducky left a comment

Choose a reason for hiding this comment

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

With method documentation, the comment should begin with the actual name of the method/function itself.

amqp.go Outdated Show resolved Hide resolved
exchanges.go Outdated Show resolved Hide resolved
queues.go Outdated Show resolved Hide resolved
Copy link
Contributor

@javaducky javaducky left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for getting those updates!

@javaducky javaducky merged commit 9d267e4 into grafana:main Aug 2, 2023
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

per-VU connection/connection management
3 participants