Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Service Bus] (Minor) Internal rename #13390

Closed
HarshaNalluru opened this issue Jan 26, 2021 · 7 comments
Closed

[Service Bus] (Minor) Internal rename #13390

HarshaNalluru opened this issue Jan 26, 2021 · 7 comments
Labels
Client This issue points to a problem in the data-plane of the library. good first issue This issue tracks work that may be a good starting point for a first-time contributor help wanted This issue is tracking work for which community contributions would be welcomed and appreciated Service Bus

Comments

@HarshaNalluru
Copy link
Member

HarshaNalluru commented Jan 26, 2021

As suggested at #13374 (comment),

_batchingReceiverLite.close can be better named as _batchingReceiverLite.cancel or _batchingReceiverLite.terminate since they seem to make more sense than close.

More code pointers:

  • Set up environment and run rush build
  • Open the sdk/servicebus/service-bus folder in your editor/IDE, search for the relevant method at src/core/batchingReceiver.ts
@ghost ghost added the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Jan 26, 2021
@HarshaNalluru HarshaNalluru added Client This issue points to a problem in the data-plane of the library. good first issue This issue tracks work that may be a good starting point for a first-time contributor help wanted This issue is tracking work for which community contributions would be welcomed and appreciated Service Bus labels Jan 26, 2021
@ghost ghost removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Jan 26, 2021
@ShrillShrestha
Copy link
Contributor

Hi there, I am a beginner and searching for some issues to learn and contribute. I am not fully knowledgeable about the project, but this seems a good issue to work on and learn. If anyone can guide me with the issue, I might be able to resolve it. Does this issue only needs renaming to be solved?

@HarshaNalluru
Copy link
Member Author

Yes, this issue is meant to rename and update with the new name wherever it is referenced.
BatchingReceiverLite class which is not part of the exported API and is used internally, has the close method - which we want to rename.

Please follow the links posted in the description for the instructions. Let me know if you're stuck at any step, I'll be able to help you through it.

@ShrillShrestha
Copy link
Contributor

Sure @HarshaNalluru, I will try to work on it. If I have any questions, I will let you know.

@ShrillShrestha
Copy link
Contributor

@HarshaNalluru, so I changed:

close(connectionError?: Error | AmqpError) {

to: terminate(connectionError?: Error | AmqpError) {.
The close method was referenced at:
this._batchingReceiverLite.close(connectionError);

which I changed to: this._batchingReceiverLite.terminate(connectionError); .
I tried to find if the method was referenced at other places, but couldn't find it at any other place. So, should I search for anywhere specific? And are the changes made above correct?

@ShrillShrestha
Copy link
Contributor

Also, how do I run a test to know if I broke anything? I tried doing rushx test as mentioned in the contribution guide, but it seems like there is no test command.
This is the error message I get:
Error: The command "test" is not defined in the package.json file for this project.
and mentions the available commands are "build", "check:everything", "latest", "local", "preinstall".

@HarshaNalluru
Copy link
Member Author

HarshaNalluru commented Jan 27, 2021

but it seems like there is no test command.

Based on the error message you posted, you might be running the command from the root of the repository.
You need to navigate to the project's folder before running the commands, sdk/servicebus/service-bus level in this case.
You can see the available commands that you can run with rushx at sdk/servicebus/service-bus/package.json.

The test command runs the live tests which would require Azure resources.
unit-test command would run the tests that don't need the Azure resources.

I tried to find if the method was referenced at other places, but couldn't find it at any other place. So, should I search for anywhere specific? And are the changes made above correct?

There could be other places.
Try build and build:test commands as they would point out the errors from the compilation.

You can open a PR with what you think is right, the team will review and request changes as needed.

ghost pushed a commit that referenced this issue Jan 28, 2021
Renamed `close` to `terminate` to resolve issue #13390.
@HarshaNalluru
Copy link
Member Author

Done with #13419

openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-js that referenced this issue Mar 29, 2021
Dev sql microsoft.sql 2020 11 01 preview minor change (Azure#13689)

* Adds base for updating Microsoft.Sql from version preview/2020-08-01-preview to version 2020-11-01-preview

* Updates readme

* Updates API version in new specs and examples

* Add updated 2020-11-01-preview versions of LongTermRetenionBackups.json and Databases.json (Azure#12146)

* add updated databases.json swagger

* update readme with new package

* v4 -> v5

* add missing chunk in readme

* add correct swaggeR

* add examples

* add correct examplese

* remove unreferenced examples

* update swagger and examples. add 2020-11 pure package

* Fix reference to 2020-08-01 reference to ElasticPools.json

* update Databases.json name

* update examples

* remove nonexistant swagger

* fixed reference capitals in readme

* update package composite and fix ref to ManagedInstances.jsON

* make backupstorageredundancy settings consistent

* add requestedBackupStorageRedundancy to properties object in example

* add LTR swagger and examples

* add LTR Policies swagger and examples

* make v5 match v4

* add updated Databses.json

* merge conflicts in LTR Policies swagger'

* take out comments in readme

* taking ImportExport.json out from 2020-11 package

* remove duplicate 2020-11 tag in readme

* modify Databases_legacy.json to exclude ImportExport-related APIs

* add servers.json for IE APIS and remove unused databases examples

* fix model & prettier errors; add missing server example

* add missing servers examples

* fix example to exclude property from server obj

* change RecommendedActions definitoins

* change tag to composite v4

* Add missing properties for api version 2020-02-02-preview of securityAlertPolicies (Azure#13117)

* Add missing properties for api version 2020-02-02-preview

* fix checks

* fix according to prettier check

* Add the change to version 2020-08-01-preview and version 2020-11-01-preview

* remove changes in package-lock

* Add new line at the end of file

* update version 2020-11-01-preview

* Update the ref to system data in serverdevopsaudit

* add missing point

* fix the validation errors

* Delete ServerDevOpsAudit.json

* fix the validation errors

* re add ServerDevOpsAudit.json

* fix the validation errors (Azure#13325)

* fix the validation errors

* Delete ServerDevOpsAudit.json

* fix the validation errors

* re add ServerDevOpsAudit.json

* address PrivateEndpointConnections.json validation issues in v3, v2 and v1

* Address Private Endpoint validation issues in V3, V2 and V1 SDK tags (Azure#13404)

* fix the validation errors

* Delete ServerDevOpsAudit.json

* fix the validation errors

* re add ServerDevOpsAudit.json

* address PrivateEndpointConnections.json validation issues in v3, v2 and v1

* Identity/Key/external Admin API (Azure#13411)

* Add examples for sql mi remove maintenance configuration (Azure#13390)

* Add missing specs 2020 11 01 (Azure#13488)

* fix the validation errors

* Delete ServerDevOpsAudit.json

* fix the validation errors

* re add ServerDevOpsAudit.json

* address PrivateEndpointConnections.json validation issues in v3, v2 and v1

* add missing specs in 2020-11-01-preview

* update the validation errors

* update systemData and sync with global common/v1/types.json

* update legacy to match the latest database specs

* remove RestorableDroppedManagedInstances due to it should be internal

* Updated database extensions (Azure#13584)

Co-authored-by: Geetha Athreya <geathrey@microsoft.com>

Co-authored-by: xaliciayang <59986952+xaliciayang@users.noreply.github.com>
Co-authored-by: ayeletshpigelman <ayshpige@microsoft.com>
Co-authored-by: strehan1993 <70013163+strehan1993@users.noreply.github.com>
Co-authored-by: Ivan Kulezic <72797230+kukislav@users.noreply.github.com>
Co-authored-by: Geetha Athreya <43223869+athreya-geetha@users.noreply.github.com>
Co-authored-by: Geetha Athreya <geathrey@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. good first issue This issue tracks work that may be a good starting point for a first-time contributor help wanted This issue is tracking work for which community contributions would be welcomed and appreciated Service Bus
Projects
None yet
Development

No branches or pull requests

2 participants