sul-dlss / folio_client

Interface for interacting with the Folio ILS API.
Other
0 stars 0 forks source link

folio_client: add single line method comments #52

Closed ndushay closed 1 year ago

ndushay commented 1 year ago

Why was this change made? 🤔

i'm reviewing the folio client code and adding comments I would find useful. YMMV.

How was this change tested? 🤨

âš¡ âš  If this change has cross service impact, including data writes to shared file systems, run integration tests and/or test in [stage|qa] environment, in addition to specs. âš¡

mjgiarlo commented 1 year ago

@jmartin-sul 💬

personally, i'm not sure i prefer this to just going to look at the wrapped method, but i get the appeal and had considered adding comments like this when adding methods to the top-level API. i guess my only worry would be drift, since the wrappers are super flexible and the wrapped methods could change without requiring these wrappers to change. but also, these comments are general enough that they're unlikely to go stale, because these methods are unlikely to change radically? so shrug

Given these methods invoke methods deeper in the gem, I think I'd probably consider usage of @see instead of having the same method comment in two places, @ndushay, if you're open to it, e.g.:

# @see Inventory#fetch_external_id
def fetch_external_id(...)
# ...
ndushay commented 1 year ago

I changed the comments per Mike's suggestion.