fix: comm-job-router: remove mocha arrow functions #24799
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
Communication
customer-reported
|
Thank you for your contribution StevanFreeborn! We will review the pull request and get back to you soon. |
3 tasks
…24798) Post release automated changes for azure-container-registry
|
@bgams or @marche0133 I'm not sure why the Is there something I've done incorrectly? |
|
@bgams or @marche0133 here is the failed report artifact that was generated for the |
Minor edit to use modern syntax and simpler control flow.
Reverts Azure#23919 These changes are reverted so that "Hierarchical Partition Feature" can be released separately as a beta package to selected customers. These changes will be merged to main trunk after the 'beta testing' phase.
### Packages impacted by this PR `sdk\communication\communication-identity` ### Issues associated with this PR Azure#13005 ### Describe the problem that is addressed by this PR The existing mocha tests for the `sdk\communication\communication-identity` made use of the arrow syntax for callback functions. Mocha recommends not to do this because you lose access to the mocha context (https://mochajs.org/#arrow-functions). ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? The reason for utilizing the function keyword instead of an arrow syntax to write the callback functions in these mocha tests is to maintain access to the mocha context. ### Are there test cases added in this PR? _(If not, why?)_ No additional test cases were added in this PR as the change only required modifying existing test cases. ### Provide a list of related PRs _(if any)_ Azure#23761 - Same fix, but for the `sdk\search\search-documents` package Azure#23789 - Same fix but for the `sdk\attestation\attestation` package Azure#23835 - Same fix but for the `sdk\batch\batch` package Azure#23850 - Same fix but for the `sdk\cognitivelanguage\ai-language-conversations` package Azure#23881 - Same fix but for the `sdk\cognitiveservices\cognitiveservices-luis-authoring` package Azure#24126 - Same fix but for the `sdk\cognitiveservices\cognitiveservices-luis-runtime` package Azure#21470 - Same fix but for the `sdk\communication\communication-chat` package Azure#24746 - Same fix but for the `sdk\communication\communication-common` package Azure#24747 - Same fix but for the `sdk\communication\communication-email` package ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ **_Not applicable_** ### Checklists - [x] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - **_I don't believe this is relevant._** - [ ] Added a changelog (if necessary) - **_I don't believe this is necessary_**
…4743) Organized and edited README so that it's easier to get started with this client library and easier to find further reference docs. ### Packages impacted by this PR ### Issues associated with this PR ### Describe the problem that is addressed by this PR ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? ### Are there test cases added in this PR? _(If not, why?)_ ### Provide a list of related PRs _(if any)_ ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ ### Checklists - [ ] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [ ] Added a changelog (if necessary)
Post release automated changes for azure-arm-liftrqumulo
Post release automated changes for azure-rest-developer-devcenter
### Packages impacted by this PR - [@azure/core-rest-pipeline] ### Issues associated with this PR Azure#24758 ### Describe the problem that is addressed by this PR Relies on the deprecated `oscpu`. This uses the non-standard `globalThis.navigator.userAgentData.platform` and falls back to the deprecated `globalThis.navigator.platform`. ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? We could have removed the OS checks overall, but we can get better telemetry to keep the OS detection. ### Are there test cases added in this PR? _(If not, why?)_ ### Provide a list of related PRs _(if any)_ ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ ### Checklists - [x] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [ ] Added a changelog (if necessary)
…#24811) - ai-language-conversations does not use rollup directly so don't need to depend on these directly - the custom rollup config in communication-network-traversal is no longer needed after we move to GA version of core-tracing.
This will enable codeQL to run on all scheduled CI jobs across all services.
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#5083 See [eng/common workflow](https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/README.md#workflow) --------- Co-authored-by: Mike Harder <[email protected]>
…ize. (Azure#23987) ### Packages impacted by this PR @azure/cosmos ### Issues associated with this PR Azure#23923 ### Describe the problem that is addressed by this PR CosmosDB Items.bulk api doens't honour 2Mb cap imposed on a single batch request. With these changes if size of a batch (cumulative size of it's operations) exceeds 2Mb it is split into smaller batches before sending. ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? ### Are there test cases added in this PR? _(If not, why?)_ Yes ### Provide a list of related PRs _(if any)_ ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ ### Checklists - [ ] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [ ] Added a changelog (if necessary) --------- Co-authored-by: FAREAST\vikassingh <[email protected]>
StevanFreeborn
requested review from
schaabs,
ellismg,
kushagraThapar,
kirankumarkolli,
abkolant-MSFT,
danielav7,
anujissarMS,
ajpeacock0,
nathpete-msft,
miguhern,
whisper6284,
RoyHerrod,
LuChen-Microsoft,
petrsvihlik,
AikoBB,
maximrytych-ms,
ostoliarova-msft,
mjafferi-msft,
jsquire and
ronniegeraghty
as code owners
|
@StevanFreeborn no worries. We are having some issue internally with the build system. it also seems that you may have introduced/merged unintended commits into this PR which triggered a lot of build pipelines. I will temporarily close this PR. |
|
Yes I'm very sorry. This PR can stay closed. I'll take another run at these changes. |
|
I will see if I can fix this |
StevanFreeborn
deleted the
fix/comm-job-router-remove-mocha-arrow-functions
branch
StevanFreeborn
restored the
fix/comm-job-router-remove-mocha-arrow-functions
branch
|
@jeremymeng okay I appreciate your efforts, but if you find it is not salvageable I don't mind just abandoning the branch and PR and starting again. |
|
@StevanFreeborn I tried but failed when force-pushing. Yes, it probably would be easier to create a new PR. |
|
@jeremymeng no problem at all. Thanks for trying to sort out my mess. Lessons learned. And again apologies for the headaches. |
StevanFreeborn
deleted the
fix/comm-job-router-remove-mocha-arrow-functions
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Packages impacted by this PR
sdk\communication\communication-job-routerIssues associated with this PR
#13005
Describe the problem that is addressed by this PR
The existing mocha tests for the
sdk\communication\communication-job-routermade use of the arrow syntax for callback functions. Mocha recommends not to do this because you lose access to the mocha context (https://mochajs.org/#arrow-functions).What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
The reason for utilizing the function keyword instead of an arrow syntax to write the callback functions in these mocha tests is to maintain access to the mocha context.
Are there test cases added in this PR? (If not, why?)
No additional test cases were added in this PR as the change only required modifying existing test cases.
Provide a list of related PRs (if any)
#23761 - Same fix, but for the
sdk\search\search-documentspackage#23789 - Same fix but for the
sdk\attestation\attestationpackage#23835 - Same fix but for the
sdk\batch\batchpackage#23850 - Same fix but for the
sdk\cognitivelanguage\ai-language-conversationspackage#23881 - Same fix but for the
sdk\cognitiveservices\cognitiveservices-luis-authoringpackage#24126 - Same fix but for the
sdk\cognitiveservices\cognitiveservices-luis-runtimepackage#21470 - Same fix but for the
sdk\communication\communication-chatpackage#24746 - Same fix but for the
sdk\communication\communication-commonpackage#24747 - Same fix but for the
sdk\communication\communication-emailpackage#24797 - Same fix but for the
sdk\communication\communication-identitypackageCommand used to generate this PR:**(Applicable only to SDK release request PRs)
Not applicable
Checklists