Skip to content

Conversation

@KieranBrantnerMagee
Copy link
Member

  • Removes localization from URIs,
  • adds tons of type hints,
  • normalizes return types,
  • begins propagating class docstring into init docstring per guidelines.

…moves localization from URIs, adds tons of type hints, normalizes return types, begins propogating class docstring into init docstring per guidelines.
@KieranBrantnerMagee KieranBrantnerMagee added Service Bus Client This issue points to a problem in the data-plane of the library. labels Aug 12, 2020
@KieranBrantnerMagee KieranBrantnerMagee added this to the [2020] September milestone Aug 12, 2020
@KieranBrantnerMagee KieranBrantnerMagee self-assigned this Aug 12, 2020
@KieranBrantnerMagee
Copy link
Member Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

"""Create a ServiceBusSessionReceiver from a connection string.
:param conn_str: The connection string of a Service Bus.
:param str conn_str: The connection string of a Service Bus.
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: in some places it's

:param conn_str: The connection string of a Service Bus.
:type conn_str: str

while here it's
:param str conn_str: The connection string of a Service Bus.

we'd better be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was actually one of the other questions I posed for @annatisch , whether my understanding for our decision tree in using those was accurate; namely: prefer the terse syntax (inline) but if this would become problematic (e.g. complex type, very long signature) utilize next-line.

Copy link
Member Author

Choose a reason for hiding this comment

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

conclusion from email: if the former generates line length or formatting issues, fall back to the latter. Don't know if that means we should just always use the latter, I'd be tempted to for consistency but it's just oh so verbose. Any strong feelings?

Copy link
Contributor

@yunhaoling yunhaoling Aug 26, 2020

Choose a reason for hiding this comment

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

(my personal preference would be the "inline" as much as possible, for the sake of reducing number of lines🤣)

KieranBrantnerMagee and others added 3 commits August 20, 2020 13:21
Defensive ServiceBusErrors for internal state misalignment.
Adjust readme streaming URL to point to proper location.

Co-authored-by: Adam Ling (MSFT) <[email protected]>
add samples for streaming iter
make receiver a ReceivedMessage param via kwargs and an expressive error to deincentivise use, but still required.
@KieranBrantnerMagee
Copy link
Member Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@KieranBrantnerMagee
Copy link
Member Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@KieranBrantnerMagee
Copy link
Member Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

yunhaoling
yunhaoling previously approved these changes Aug 26, 2020
Copy link
Contributor

@yunhaoling yunhaoling left a comment

Choose a reason for hiding this comment

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

LGTM

…ply always pass receiver in kwargs (since it's _receiver_ mixins anyway)
@KieranBrantnerMagee
Copy link
Member Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@KieranBrantnerMagee
Copy link
Member Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@KieranBrantnerMagee KieranBrantnerMagee merged commit cda14a3 into Azure:master Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue points to a problem in the data-plane of the library. Service Bus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants