[core-amqp] Fixing typos

[sadasant]

I know this PR is kind of obnoxious, but I thought I could help.

I was able to detect these by using Code Spell Checker on VSCode.

[richardpark-msft]

NOTE: Wait to merge until we hear back from the others on the wording changes for the log messages.

Thanks for doing this though, there were some pretty egregious ones there!

[richardpark-msft]

So here's a funny situation. If a customer sends us logs and we go and search for the message in our code we won't be able to find it anymore.

Might not be a big deal as long as we're aware that misspellings have been corrected :)

CC: @ramya-rao-a , @chradek , @HarshaNalluru

[ramya-rao-a]

If this was for @azure/amqp-common, then I would have the same concerns. This is because the number of existing customers using Track 1 EH & SB in production would be much larger than the ones for Track 2 at this point in time. Right now, this concerns customers who have EH Track 2 in production, and am ok to bite the bullet and get it done.

[sadasant]

@richardpark-msft I wonder if this could break anything else.

[ramya-rao-a]

Yes, this would be a breaking change. If we are doing this, then we will have to update the major version to 2

[richardpark-msft]

Gah, the curse of having an "internal" library be externally available.

Everything else in this PR is goodness, let's just leave this change out for now.

[chradek]

I don't know...I kind of see it as a bug fix. I think the fact that no one has reported that it's typo'd as satusDescription is some indicator that probably no one is using this field yet. If we really wanted, we could create an alias in JS so anyone using that field at runtime would still see it. Though having done something like that in the past people will wonder why they both show up...

[bterlson]

I think we should fix this - it's not super clear that this is typo'd, I might just not even realize what it is.

There is a hybrid approach here - we can support both, but have the typings (and docs) only show latest. That way JS users don't need to do anything, and TS builds break but the fix is easy - it will offer a "did you mean" error since the string is only off-by-one, and the user could just cast to any without any code changes to make the error go away and things work... or even if they just ignore the error, the emitted code will continue to work also.

[sadasant]

Also, what do you all think of this message? CbsResponse's property "satusDescription" is deprecated, use "statusDescription" instead.

[bterlson]

If today you have:

let obj = {
  ...
  satusDescription: str
}

you would change it to

let obj = {
  ...
  statusDescription: str
}

Object.defineProperty(obj, 'satusDescription', {
  get() {
    logger.warning(`CbsResponse's property "satusDescription" has been renamed to "statusDescription". "satusDescription" will be removed in v2.0.0.`)
    return this.statusDescription;
  },
  enumerable: false
});

and make sure you add @deprecated to the field's type definition.

[ramya-rao-a]

While taking this approach makes sense for when we want to deprecate properties, there is not much of a return on investment here. There most likely is no user scenario to even expose the statusCode and the statusDescription in this case.

For the scope of this PR, I would recommend not making any change to satusDescription. @richardpark-msft, I would recommend we log an issue to investigate the usefulness of the status code & description when getting back the response of a claim negotiation. I am sure that a deeper dive into core-amqp may bring other things we want to change in a v2

[sadasant]

Alright, I'm undoing that specific change then.

@bterlson Thank you for that though! I haven't used defineProperty in ages 😱 amazing.

[sadasant]

@richardpark-msft Here's an issue! #9031 Good luck 🌞

[ramya-rao-a]

Either revert the change from satusDescription to statusDescription or update the major version to 2.

I'll leave the decision to @chradek, @richardpark-msft and @HarshaNalluru