-
Notifications
You must be signed in to change notification settings - Fork 1
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
Return unknown operation_details in parser #26
Return unknown operation_details in parser #26
Conversation
3f73c2b
to
b6fef0c
Compare
97641f3
to
d9cb64a
Compare
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.
Nice 👌
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.
Nice work 🔥 Just a tiny comment :)
README.md
Outdated
@@ -33,8 +33,9 @@ gem 'cfonb' | |||
Below is the list of additional details available for each operation. | |||
|
|||
These details can be accessed through `operation.details`, which will provide all the attributes. To fetch a specific attribute, you can use `operation.details.attribute`. For example, `operation.details.unstructured_label`. Ultimately, you can also access the 70 characters of the detail by using its code like `operation.details.mmo` | |||
All unmapped details can be accessed via `details.unknown` which will return the codes and the corresponding line details. |
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 would add an example of how it can look like and how to access a specific code
@@ -15,7 +15,9 @@ def self.register(code, klass) | |||
end | |||
|
|||
def self.for(line) | |||
@details[line.detail_code] | |||
return unless line.respond_to?(:detail_code) |
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.
You can avoid this no by making returning an Unknown class for each unknown code ?
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.
We do that below, this was to prevent cases where the file might be corrupted and we didn't have a detail_code at all (since we need the detail code for both know known and unknown classes). Maybe I should raise instead?
@@ -15,7 +15,9 @@ def self.register(code, klass) | |||
end | |||
|
|||
def self.for(line) | |||
@details[line.detail_code] | |||
return unless line.respond_to?(:detail_code) |
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.
Normally an unknown should respond to detail code
So this seems not relevant
No description provided.