Skip to content

Refactor and simplify returns #574

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

Merged
merged 2 commits into from
Sep 6, 2018
Merged

Conversation

CyborgMaster
Copy link
Contributor

Refactor and simplify returns for annotate_one_file. This simplifies some big if/else blocks that were wrapping significant portions of this method. No logic is changed.

This is best viewed hiding whitespace changes (under the "Diff settings" button).

This is currently built on top of #573, because it hits the same lines of code (I split it out because the whitespace changes make it hit every line in the function). If for some reason you want to merge this one and not #573, I'll be happy to refactor.

@ctran ctran self-assigned this Sep 6, 2018
@ctran ctran added this to the v.2.7.5 milestone Sep 6, 2018
@CyborgMaster
Copy link
Contributor Author

After you get 573 merged, I'll rebase this one too and hopefully that will make the diff simpler.

@CyborgMaster
Copy link
Contributor Author

Rebased! If you hide the whitespace changes now, the diff is quite short.

@ctran ctran merged commit ca4090d into ctran:develop Sep 6, 2018
@ctran
Copy link
Owner

ctran commented Sep 6, 2018

Thanks!!!

@CyborgMaster CyborgMaster deleted the refactor-returns branch September 6, 2018 17:06
@CyborgMaster
Copy link
Contributor Author

You're welcome. Thanks for getting these merged so fast. I don't think I've seen such a quick turn around on an OSS project before. Much appreciated.

mgpnd pushed a commit to mgpnd/annotate_models that referenced this pull request Feb 15, 2019
* refactor and simplify returns for annotate_one_file
* fix rubocop offenses
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