Skip to content

glossary alignment#40

Merged
DT-DawidWroblewski merged 9 commits intocamaraproject:mainfrom
monamok:feat/glossary_alignment
Jun 15, 2023
Merged

glossary alignment#40
DT-DawidWroblewski merged 9 commits intocamaraproject:mainfrom
monamok:feat/glossary_alignment

Conversation

@monamok
Copy link
Contributor

@monamok monamok commented Jun 6, 2023 •
edited
Loading

What type of PR is this?

  • fix

What this PR does / why we need it:

The parameters naming alignment with Glossary

Which issue(s) this PR fixes:

Fixes (#41)

Special notes for reviewers:

  • The parameters names were indicated in Glossary in order to be unified in the whole CAMARA project. This PR is to change msisdn to phoneNumber.
  • Also the example of phoneNumber was change to make it more clear that country code must be included.
  • Some typo fixes and minor changes were included

Changelog input

Changelogs will be included once #28 in accepted and merged

Additional documentation

@monamok monamok mentioned this pull request Jun 6, 2023
Closed
bigludo7
bigludo7 previously approved these changes Jun 13, 2023
View reviewed changes
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

Works for me.
We will introduce the UTC precision later?

@monamok
Copy link
Contributor Author

monamok commented Jun 13, 2023

We will introduce the UTC precision later?

We can include it in the PR we have to open for #42

@bigludo7 bigludo7 self-requested a review June 13, 2023 15:18
bigludo7
bigludo7 reviewed Jun 13, 2023
View reviewed changes
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

Added UTC precision + error code 404 to accomodate both issue open.

@bigludo7 bigludo7 mentioned this pull request Jun 13, 2023
Closed
@monamok monamok requested a review from bigludo7 June 14, 2023 12:59
@monamok
Copy link
Contributor Author

monamok commented Jun 14, 2023

Added UTC precision

@bigludo7 are we going to wait for the commonalities issue about this to get resolved or are we going to merge this PR and then align it with the decision afterwards?

@bigludo7
Copy link
Collaborator

bigludo7 commented Jun 14, 2023

@monamok I suggest to wait tomorrow Commonalities call.
If the team there pick an option during the call we can accommodate our api accordingly.
If no decision, we remove the UTC precision from our API and we merge without it.

Probably good to inform @DT-DawidWroblewski. I should not be able to connect tomorrow to the Commonalities call so hope someone can trigger the discussion (Dawid, you or @gregory1g )
Work for you?

@monamok
Copy link
Contributor Author

monamok commented Jun 14, 2023

@bigludo7 ok, we can wait to commonalities meeting.
Should I include what we talked in #42 about removing Client Credentials and leave only 3-legged in this PR too? Or do you prefer to open a new PR for that?

@monamok
Copy link
Contributor Author

monamok commented Jun 14, 2023

@bigludo7 I removed UTC for the moment so if you approve and @DT-DawidWroblewski agrees too, we can merge and close this PR tomorrow during the bi-weekly meeting.
For the other two issues I will open a new PR afterwards.

bigludo7
bigludo7 previously approved these changes Jun 14, 2023
View reviewed changes
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

OK work for me as well.
We can create another PR for UTC after.
Let's go for this

@monamok
Copy link
Contributor Author

monamok commented Jun 14, 2023

@bigludo7 thank you.

DT-DawidWroblewski
DT-DawidWroblewski approved these changes Jun 15, 2023
View reviewed changes
Copy link
Collaborator

@DT-DawidWroblewski DT-DawidWroblewski left a comment

Choose a reason for hiding this comment

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

all good

@DT-DawidWroblewski DT-DawidWroblewski merged commit d0e6d32 into camaraproject:main Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bigludo7 bigludo7 bigludo7 left review comments

+1 more reviewer

@DT-DawidWroblewski DT-DawidWroblewski DT-DawidWroblewski approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@monamok @bigludo7 @DT-DawidWroblewski