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

Rename properties to new terms #129

Merged
merged 4 commits into from
Apr 14, 2023

Conversation

jlurien
Copy link
Collaborator

@jlurien jlurien commented Mar 9, 2023

  • Applied lowerCamelCase for property names, UpperCamelCase for schema names
  • Updated API_documentation as well. This is a redundant effort that hopefully we don't have to maintain in the future, as being discussed in Revisit API documentation template to reduce redundancy WorkingGroups#164
  • In a separate PR we should enhance descriptions, once Glossary definitions are consolidated.

- Applied lowerCamelCase for property names, UpperCamelCase for schema names
- Updated API_documentation as well. This is a redundant effort that hopefully we don't have to maintain in the future, as being discussed in camaraproject/WorkingGroups#164
- In a separate PR we should enhance descriptions, once Glossary definitions are consolidated.
@hdamker
Copy link
Collaborator

hdamker commented Mar 9, 2023

We should wait with this PR until camaraproject/WorkingGroups#157 is finally decided with a vote as requested by Telefonica.

hdamker
hdamker previously approved these changes Mar 24, 2023
@hdamker
Copy link
Collaborator

hdamker commented Mar 24, 2023

Guess we are ready to due these changes now as the guidelines are also updated.

@eric-murray @jlurien @patrice-conil @sfnuser @akoshunyadi would you have a look and approve as well?

@@ -63,41 +63,41 @@ paths:
code: INVALID_ARGUMENT
message: "Schema validation failed at ..."
MsisdnRequired:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we update this line too?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would say so.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done in ed3bbdc

@@ -26,14 +26,14 @@ Security access keys such as OAuth 2.0 client credentials used by client applica
**QoS profiles and QoS profile labels**
Latency or throughput requirements of the application mapped to relevant QoS profile class.

**Identifier for the the user equipment (UE)**
At least one identifier for the user equipment out of four options: IPv4 address, IPv6 address, MSISDN, or external ID [[5]](#5) assigned by the mobile network operator for the user equipment
**Identifier for the the device**
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: the the

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done in fe2cef9

**Identifier for the the user equipment (UE)**
At least one identifier for the user equipment out of four options: IPv4 address, IPv6 address, MSISDN, or external ID [[5]](#5) assigned by the mobile network operator for the user equipment
**Identifier for the the device**
At least one identifier for the device (user equipment) out of four options: IPv4 address, IPv6 address, Phone number, or Network Access Identifier [[5]](#5) assigned by the mobile network operator for the device.

**Identifier for the application server (AS)**
Copy link
Collaborator

Choose a reason for hiding this comment

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

AS is not used anymore, more occurrences are affected

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done in fe2cef9 (plus replaced one remaining occurence of UE).

@hdamker
Copy link
Collaborator

hdamker commented Apr 5, 2023

@jlurien will you resolve the comments or should I?

MsisdnRequired replaced with PhoneNumber required.
Corrected type and remaining occurences of AS and UE.
@hdamker
Copy link
Collaborator

hdamker commented Apr 13, 2023

AS @jlurien seems not be available I have addressed the comments above.
@akoshunyadi and @sfnuser can you shortly check and approve?

@sfnuser
Copy link
Collaborator

sfnuser commented Apr 13, 2023

AS @jlurien seems not be available I have addressed the comments above. @akoshunyadi and @sfnuser can you shortly check and approve?

@hdamker - Cheers. Are we planning to update the commit in two other places (line 83 & 95) as well?

Further updates of error examples in lines 71, 77, 83, 89, 95
@hdamker
Copy link
Collaborator

hdamker commented Apr 13, 2023

@sfnuser :

Are we planning to update the commit in two other places (line 83 & 95) as well?

Thanks for the hint ... have done further updates of error examples in lines 71, 77, 83, 89, 95 to get all the error examples consistent with the new property terms. Please have a view.

@sfnuser
Copy link
Collaborator

sfnuser commented Apr 13, 2023

@sfnuser :

Are we planning to update the commit in two other places (line 83 & 95) as well?

Thanks for the hint ... have done further updates of error examples in lines 71, 77, 83, 89, 95 to get all the error examples consistent with the new property terms. Please have a view.

Thanks. Looks good to me.

value:
status: 400
code: INVALID_ARGUMENT
message: "Expected property is missing: qos"
message: "Expected property is missing: qosProfile"
UePortsRangesNotAllowed:
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hdamker - are we planning to update this one too?

value:
status: 400
code: INVALID_ARGUMENT
message: "Expected property is missing: ueId.ipv4addr or ueId.ipv6addr"
message: "Expected property is missing: device.ipv4Address or device.ipv6Address"
UePortsRequired:
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hdamker - are we planning to update this one too?

Copy link
Collaborator

@hdamker hdamker left a comment

Choose a reason for hiding this comment

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

Hopefully we have now found all occurence which need to be replaced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants