fix(spec): name conflict between method and params #121
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
shortcuts
changed the title
eunjae-lee
reviewed
Feb 7, 2022
| */ | ||
| function batchDictionaryEntries({ | ||
| dictionaryName, | ||
| batchDictionaryEntries, | ||
| batchDictionaryEntriesParams, |
There was a problem hiding this comment.
I might be too late for this discussion, but I'm wondering if we cannot have flattened parameters like:
dictionaryName: 'compounds' | 'plurals' | 'stopwords';
clearExistingDictionaryEntries?: boolean;
requests: BatchDictionaryEntriesRequest[];It's weird to have a nested parameter for whatever benefit I cannot see 🤔
There was a problem hiding this comment.
The goal of wrapping parameters in an object is to have a single parameter when calling the method. It would indeed improve the DX to flatten this object but adds a lot of complexity to our templates (might be worth doing a small POC, I'll take a closer look this week).
This is definitely an open discussion as we're still in closed beta, any improvements are welcome!
There was a problem hiding this comment.
Sure! Let's keep it at the back of our brains :)
shortcuts
changed the title
shortcuts
added a commit
that referenced
this pull request
Apr 22, 2022
This was referenced May 24, 2022
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.
🧭 What and Why
🎟 JIRA Ticket: -
Changes included:
We've decided to go with the
Paramssuffix in our specs after #96, but some of the parameters were still using other wording. It's not mandatory to use theParamssuffix, but it should be the go-to when the name conflict with theoperationId.🧪 Test
CI :D