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/new version calculator #28

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Refactor/new version calculator #28

wants to merge 2 commits into from

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