Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Valkyrie::StorageAdapter documentation correction? #951

Closed
dchandekstark opened this issue Apr 24, 2024 · 1 comment
Closed

Valkyrie::StorageAdapter documentation correction? #951

dchandekstark opened this issue Apr 24, 2024 · 1 comment
Labels
documentation question

Comments

@dchandekstark
Copy link
Member

The documentation on Valkyrie::StorageAdapter -- at

valkyrie/lib/valkyrie/storage_adapter.rb

Line 28 in ccb2677

# @return [Valkyrie::StorageAdapter]
and

valkyrie/lib/valkyrie/storage_adapter.rb

Line 56 in ccb2677

# @return [Valkyrie::StorageAdapter]
-- suggests that adapters are instances of that class, although they are not. (I'm not sure why adapters shouldn't subclass V::SA, but apparently things went a different way?) Is a documentation correction warranted?
@dchandekstark dchandekstark changed the title Valkyrie::StorageAdapter interface? Valkyrie::StorageAdapter documentation correction? Apr 24, 2024
@tpendragon
Copy link
Collaborator

suggests that adapters are instances of that class, although they are not. (I'm not sure why adapters shouldn't subclass V::SA, but apparently things went a different way?) Is a documentation correction warranted?

Seems reasonable!

dchandekstark added a commit to dchandekstark/valkyrie that referenced this issue Apr 24, 2024
@dchandekstark
Addresses documentation issue in samvera#951
014186d 
Also:
- Corrects the documentation of Valkyrie::StorageAdapter::File#valid?
- Implements AdapterNotFoundError as documented on Valkyrie::StorageAdapter.find
  and in test description. (I'm not sure if this would technically be a breaking
  change since the raised exception would not be a RuntimeError.)
tpendragon added a commit that referenced this issue Apr 24, 2024
@tpendragon
Merge pull request #952 from dchandekstark/issue-951-docs
e2f3a20 
Addresses documentation issue in #951
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
documentation question
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dchandekstark @tpendragon