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

Refactor CLI & Configuration #672

Merged
merged 9 commits into from
Mar 15, 2015
Merged

Refactor CLI & Configuration #672

merged 9 commits into from
Mar 15, 2015

Conversation

chulkilee
Copy link
Contributor

  • Made internal methods private: e.g. parse_options
  • Extracted methods: e.g. setup_binds, close_binder_listeners
  • Extracted file: dsl.rb for Puma::DSL (originally in configuration.rb)
  • Moved Puma::cli_config from configuration.rb to cli.rb
  • Reordered methods for readability
  • Improved code readability
    • avoid assignment in if condition
    • avoid name conflicts between local variable and instance method
    • shorter block
    • use each_with_object
    • each over each_with_index when index is not used
    • and so on...

Only the first thing (making methods private) will break other apps using them - but from my understanding those methods shouldn't be called outside of puma, so I just made the change.

I started this work as part of #669 and found it would be better to merge refactoring before adding features. Please take a look and please give some feedback.

@chulkilee chulkilee mentioned this pull request Mar 15, 2015
2 tasks
evanphx added a commit that referenced this pull request Mar 15, 2015
Refactor CLI & Configuration
@evanphx evanphx merged commit 9d0567d into puma:master Mar 15, 2015
@evanphx
Copy link
Member

evanphx commented Mar 15, 2015

Thanks @chulkilee! Great work!

@chulkilee chulkilee deleted the refactor branch October 29, 2022 00:51
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.

None yet

3 participants