-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add common exception class #30
base: master
Are you sure you want to change the base?
Conversation
- Allows clients to rescue the common ServiceError class if they don't care what error the cmis service raised
@@ -1,18 +1,19 @@ | |||
module CMIS | |||
module Exceptions | |||
InvalidArgument = Class.new(StandardError) |
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.
How about ordering them alpabetically? Would make things more maintainable...
@@ -1,18 +1,19 @@ | |||
module CMIS | |||
module Exceptions |
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.
Get rid of the exceptions module, it just adds complexity.
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.
I'd rather not, as this will surely break backwards compatibility with certain clients of this lib that rely on rescuing these exceptions.
Also, by removing the Exceptions
namespace, assuming we still want to enforce a common parent class for all CMIS service related exceptions/errors, we would have to write CMIS::StorageException
iso CMIS::Exceptions::Storage
, which to me, is the same thing.
If you feel that strongly about removing the Exceptions
namespace a compromise would be to introduce a new exception parent for each exception that sits between ServiceError
and the exception.
For example
module CMIS
ServiceError = Class.new(StandardError)
StorageException = Class.new(ServiceError)
module Exceptions
Storage = Class.new(StorageException)
...
care what error the cmis service raised