-
Notifications
You must be signed in to change notification settings - Fork 39
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
Enum descriptions #13
Comments
@phoenixy1 perhaps we could add descriptions to these? |
@viceroypenguin OpenAPI doesn't seem to support adding descriptions to enums. From https://swagger.io/docs/specification/data-models/enums/ "If you need to specify descriptions for enum items, you can do this in the description of the parameter or property". Is the request to enumerate all the possible enums in the description field? |
@phoenixy1 - Only looking for descriptions similar to other enums already present in the .yaml file, such as for |
@viceroypenguin OK, so it sounds like the request is that the descriptions should be formatted like they are for ACHClass, in order to make it easier to parse them? AccountSubtype and BankTransferType for example do have descriptions, but they are formatted differently. To go through these one by one because they are all slightly different: AccountSubtype: I don't think we can change this one to match that format, it would take up waaaay too much space in the documentation. BankTransferStatus: It might be worth changing this, the fields are mostly self-explanatory but not totally. BankTransferType: If the ask is just to reformat the description, I can easily do that. BankTransferNetwork, CountryCode, Products: Are you already parsing the list of possible enum values and putting that in the docs? (Our API docs currently do that.) If you are, I'm not sure it would add value to do that for some of these enums that are totally (or (99%) self-explanatory and where we have no real descriptive information to add. BankTransferEventType: Not sure I understand the request here -- the description seems to be in the right place, as far as I can tell? |
AccountSubtype: The reason for the ask is to (in line with the vision of providing the openapi) eventually automate generating the entities used in the supporting library. Currently I am parsing the documentation directly from the .yaml file to apply to the enum values directly, so that users will have information present in the code as they use it. However, the AccountSubtype exception would mean that for each upgrade I would need to manually go to the plaid docs and provide documentation for new entries (such as recently added life insurance, etc.). Alternatively, I could leave documentation as simple the enum value and a link to the plaid website, but that seems like an extra step. Is there any other place where the AccountSubtype information could be automatically collected and applied. Actually, given the nature of AccountSubtype having a different documentation nature, maybe that would be best anyway? BankTransferStatus, BankTransferType: Yes please BankTransferNetwork, CountryCode, Products: I am currently parsing the description field of the enum definition and extracting documentation of the enum values and applying them to the values directly. Understood there may not be more information to describe these than already present, but it would still be useful to have available. For example, the comment on Products about the "Balance" product would be useful on the Balance enum value, etc. This would be especially important if the descriptions change in the future. BankTransferEventType: Yes, the enum values are described, but there is no description for the enum type itself. Even something as simple as "The type of event that this bank transfer represents" or similar, same as you do for other enum types. Let me know if these aren't clear enough or if could explain better? |
AccountSubtypes: These are defined in the members of the StandaloneAccountType schema object, is there anything useful you can do with that? (We created this object as a slight abuse of the OpenAPI format in order to make pretty tables in the API docs to show account types and subtypes.) BankTransferStatus, BankTransferType: Will do. BankTransferNetwork, CountryCode, Products: Will take this back to the team to think about...for stuff like the Products and CountryCode enum it's basically a tradeoff between making the information easier to parse in an automated way and taking up extra space on the API Reference docs page. BankTransferNetwork is no big deal because there are only three but for products there are like 12 so adding descriptions for all of them makes me wince a little. BankTransferEventType: Oh, that's all you needed? Got it. Commit in progress! |
StandaloneAccountType - I'll definitely take a look at this. It'll probably get me what I need. I just have to figure out how to special-case this one. Thanks! |
@phoenixy1 Further question - there are other enums throughout the system that are not created as separate enums like AccountType, Products, etc. Several examples of these in the
If not, I'll look at other ways to parse these into separate enums on my side. Thanks! |
@phoenixy1 PS: Thanks for the updates you've done already for BTS, BTT, etc. |
@viceroypenguin hmm...is it important for these to be separate enums? generally we've been separating stuff out into separate objects it's reused somewhere but doing them as one-offs if it's not. we can do this if there aren't too many of them, it would be helpful for you to identify them. |
I'll review and get back to you. I figured out how to render them, so it may just be a matter of titling them in some cases for cleaner type-naming. |
@viceroypenguin we are considering removing enums from the openapi file altogether because we consider enums extensible but it turns out that the openapi spec does not support extensible enums. We fixed it for our own libraries by removing validation on enums but are worried it might bite community developers like you. Do you have any opinion on this one way or another? |
@phoenixy1 Yes, removing enums would make life difficult for C# developers at least. Take the With enums, I can specify as the library developer what the allowed values are, and the consumer coder can know the range of potential values and write code to provide those values quickly and efficiently. It also reduces the amount of trial and error from users forgetting the value, mis-typing the value, or mis-understanding the value. Without enums, these values would be There are two possibilities on our side to deal with it, neither of which is as clean as enums: we could specify a constants table and encourage (but not require) consumers to use the table; or we could include the range of values in the comments of each field. Either of these options could reduce the number of errors, but neither is as effective at early detection of invalid values as enums are. Having said all of the above, I'll counter-argue: if the enum properties are supposed to be extensible (i.e. you may add values on the fly), then yes, enums would create validation issues if a new value is provided that is not in the list of expected potential values. Maybe a compromise? Any properties that consumers are expected to provide could be enums, but other properties just have a non-exclusive description of potential values. This would allow enforcement of proper set of values for input to the Plaid api, but allow flexibility in the output of the Plaid api. This would not be restrictive, because if a new value is allowed for input, then the next library update can include the new values, but no existing code will be hurt; but if new values are allowed for output, the consuming code will know to have an exception case for unexpected values and deal with them appropriately (whether pass-thru for display or ignore the response or whatever makes sense for the particular property). |
The following enums do not have descriptions:
While most of these are fairly self evident, it makes it more difficult to add documentation on generated code.
The text was updated successfully, but these errors were encountered: