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.

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

Reactors AnnotateModels.get_schema_info #791

Merged
merged 1 commit into from
Apr 5, 2020

Conversation

tmr08c
Copy link
Contributor

@tmr08c tmr08c commented Apr 4, 2020

This is a bit of a cheat of a refactoring that simply extracts the logic for collecting a column's attributes out of get_schema_info and into its own method (get_attributes).

I found that in PRs like #779 that the Rubocop ABC limit was being exceeded:

lib/annotate/annotate_models.rb:235:5: C: Metrics/AbcSize: Assignment Branch Condition size for get_schema_info is too high. [145/145]
    def get_schema_info(klass, header, options = {}) ...
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Hopefully, this should break this up and reduce the complexity of the method.

There are opportunities to go further, but I thought this could be a good place to start.

I would be open and interested in discussing further refactoring opportunities if it would make sense (maybe creating some new classes to encapsulate some of this logic).

Extracts the logic for collecting a column's attributes into its own
method.
@tmr08c tmr08c force-pushed the tmr08c-reduce-abc-for-get-schema-info branch from 1880169 to 17e3dc3 Compare April 4, 2020 01:24
Copy link
Collaborator

@drwl drwl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, this change should fix the rubocop issue in the other PR. Thanks for this!

@drwl drwl merged commit da1e605 into ctran:develop Apr 5, 2020
@tmr08c tmr08c deleted the tmr08c-reduce-abc-for-get-schema-info branch April 5, 2020 14:50
vfonic pushed a commit to vfonic/annotate_models that referenced this pull request May 8, 2020
This is a bit of a cheat of a refactoring that simply extracts the logic for collecting a column's attributes out of `get_schema_info` and into its own method (`get_attributes`).

I found that in PRs like ctran#779 that the Rubocop ABC limit was being exceeded:

```
lib/annotate/annotate_models.rb:235:5: C: Metrics/AbcSize: Assignment Branch Condition size for get_schema_info is too high. [145/145]
    def get_schema_info(klass, header, options = {}) ...
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

Hopefully, this should break this up and reduce the complexity of the method.

There are opportunities to go further, but I thought this could be a good place to start. 

I would be open and interested in discussing further refactoring opportunities if it would make sense (maybe creating some new classes to encapsulate some of this logic).
ocarta-l pushed a commit to ocarta-l/annotate_models that referenced this pull request Jun 18, 2021
This is a bit of a cheat of a refactoring that simply extracts the logic for collecting a column's attributes out of `get_schema_info` and into its own method (`get_attributes`).

I found that in PRs like ctran#779 that the Rubocop ABC limit was being exceeded:

```
lib/annotate/annotate_models.rb:235:5: C: Metrics/AbcSize: Assignment Branch Condition size for get_schema_info is too high. [145/145]
    def get_schema_info(klass, header, options = {}) ...
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

Hopefully, this should break this up and reduce the complexity of the method.

There are opportunities to go further, but I thought this could be a good place to start. 

I would be open and interested in discussing further refactoring opportunities if it would make sense (maybe creating some new classes to encapsulate some of this logic).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants