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.
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
IMN-657 GET EService descriptor by ID #712
IMN-657 GET EService descriptor by ID #712
Changes from 26 commits
a02dc84
b6d1ce8
af35559
56a0277
b1ac408
8f82ffe
87effa8
6b080a5
107daaa
17be40e
6006856
33b8648
392cb8c
43848ef
480d433
5bb768c
dfba69b
6429d36
9cf1835
7dc73a2
846effd
98a7bad
748d8c6
9fe126b
9e9d46e
3d974f2
00e0562
1b30f5a
28954da
d3d4657
5ba6040
31c5dcd
11026a3
c41d3ec
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using runtime resources to change something at type level, I know the performance impact could be negligible but I saw this in a lot of part of the monorepo and I'm wondering if it is worth discussing.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Carminepo2 good point 👍🏻
unfortunately branded type usages requires mapping like this, so we need to choose what we want encourage. Id as string are very error prone when they are function's parameter, branded type is a sort of guard to avoid bugs, meanwhile every time we convert models it's necessary this wrapping...
I suggest to discuss a possible policy offline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doubt: in the BFF do we have to use the same codes used in Scala, being exposed directly to the FE, or can we do like in the processes and redefine our codes from scratch just incrementing whenever we add a new error?
cc @galales
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline with @Carminepo2 and @beetlecrunch, this code could be different from the Scala codebase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, so let's do the increment by one since we don't need to adhere to Scala codes: