Skip to content

Refactor/new version calculator#28

Open
JuanitoFatas wants to merge 2 commits intolivingsocial:masterfrom
JuanitoFatas:refactor/new-version-calculator
Open

Refactor/new version calculator#28
JuanitoFatas wants to merge 2 commits intolivingsocial:masterfrom
JuanitoFatas:refactor/new-version-calculator

Conversation

@JuanitoFatas
Copy link
Contributor

@JuanitoFatas JuanitoFatas commented Aug 9, 2016

Hi Chris,

I am getting started with the codebase, and I find it extract to smaller class (1ca92db) can be easier to understand and easier to write tests (in my opinion). But the downsides are the cost of file jumps (for reader) and a new dependency for UpdateSpec (now depends on NewVersionCalculator).

2bbc58d is on your call, I change it so there is no need to call .dup when passing in patched_versions, so the reader won't have to worry: Oh! Something destructive goes on there.

What do you think? :)

@chrismo
Copy link
Member

chrismo commented Aug 10, 2016

NewVersionCalculator I'm probably good with. I'll soak my brain in it some more later.

@chrismo
Copy link
Member

chrismo commented Aug 24, 2016

apologies for ignoring this. since bundler 1.13.0.rc.2 is now released, this stuff is getting more of my attention, so i might get back to this soon. i'm happy for your contributions!

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