Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Refactoring config.rb #4047

Merged
merged 1 commit into from
Oct 11, 2015
Merged

Refactoring config.rb #4047

merged 1 commit into from
Oct 11, 2015

Conversation

A5308Y
Copy link
Contributor

@A5308Y A5308Y commented Oct 10, 2015

My main goal was to show the structure of the run method:

  1. Guard Clauses
  2. Message to user
  3. Save new settings

and to separate the message texts from the rest of the logic.
So the class and methods are easier to understand read.

While doing that I reduced the complexity of the run method by
extracting methods. This should improve the Code Climate rating of
Bundler::CLI::Config substantially.

My main goal was to show the structure of the run method:
1. Guard Clauses
2. Message to user
3. Save new settings

and to separate the message texts from the rest of the logic.
So the class and methods are easier to understand read.

While doing that I reduced the complexity of the run method by
extracting methods. This should improve the Code Climate rating of
Bundler::CLI::Config substantially.
@segiddins
Copy link
Member

This will probably conflict with @smlance's 2.0 work?

@A5308Y
Copy link
Contributor Author

A5308Y commented Oct 11, 2015

Hmm. Seems like Travis has stopped for some reason. Some builds never started one says something about git clone failing. Should I do some random change and push again or is there a better way to restart the build process?
A search for config.rb in a comparison between "1-99-dev" and "master" returned no results. Is that what you were thinking about, @segiddins?

@indirect
Copy link
Member

I just restarted the build. Hopefully it'll pass this time.

@A5308Y
Copy link
Contributor Author

A5308Y commented Oct 11, 2015

Thanks @indirect!

@indirect
Copy link
Member

@homu r+

@homu
Copy link
Contributor

homu commented Oct 11, 2015

📌 Commit 4b7d61b has been approved by indirect

homu added a commit that referenced this pull request Oct 11, 2015
Refactoring config.rb

My main goal was to show the structure of the run method:
1. Guard Clauses
2. Message to user
3. Save new settings

and to separate the message texts from the rest of the logic.
So the class and methods are easier to understand read.

While doing that I reduced the complexity of the run method by
extracting methods. This should improve the Code Climate rating of
Bundler::CLI::Config substantially.
@homu
Copy link
Contributor

homu commented Oct 11, 2015

⌛ Testing commit 4b7d61b with merge 98a4c88...

@segiddins
Copy link
Member

@indirect is this you volunteering to reconcile this PR with the changes on 2-0-dev?

@indirect
Copy link
Member

@segiddins yup 😆

@homu
Copy link
Contributor

homu commented Oct 11, 2015

☀️ Test successful - status

@homu homu merged commit 4b7d61b into rubygems:master Oct 11, 2015
@A5308Y
Copy link
Contributor Author

A5308Y commented Oct 12, 2015

@indirect Thanks! I hope this isn't causing too much merge pains... If I can help with the reconciliation somehow I'd be happy to.
@segiddins Thanks for pointing me to dev-2-0. I feel a bit stupid for not finding it... :-|
I enjoy doing this kind of refactorings and I'd like to do more for bundler. So far I was just looking at code climate to find D or F rated code I'd feel comfortable improving. I don't want to cause extra work for you though. Would it be better to ask around in slack before working on something?

@indirect
Copy link
Member

@A5308Y I'm very happy to work with you on refactorings :) We will have to work with this code base for a long time, so I'd rather do the work of cleaning it up and then porting those cleanups to the 2.0 branch than wait for 2.0 to be finished first. You can definitely ask in Slack, but feel free to send PRs for sure.

@A5308Y
Copy link
Contributor Author

A5308Y commented Oct 12, 2015

@indirect Cool :-) Thanks for making it so easy to get involved!

@coilysiren coilysiren modified the milestone: Release Archive Oct 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants