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

Enhancement/no map in void context #32

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@mcmillhj
Contributor

mcmillhj commented Feb 12, 2016

patch for issue#30, remove map statements that don't produce a resulting list in favor of foreach loops

mcmillhj added some commits Feb 12, 2016

replace map expressions that aren't doing list transformations
patch for issue#30, remove map statements that don't produce a resulting list in favor of foreach loops
add attribute to allow products to, optionally, not be combined by SKU
patch for issue#13:
There is now a new attribute in lib/Interchange6/Cart/Product.pm called `combine` that accepts either a boolean value (0, 1) or a coderef.
There is now logic in  lib/Interchange6/Cart.pm that examines the `combine` attribute *before* merging a products in the Cart that have the same SKU.
A new test, t/01-combine-by-sku.t tests all variations of values that can be passed to the `combine` attribute
@mcmillhj

This comment has been minimized.

Show comment
Hide comment
@mcmillhj

mcmillhj Feb 12, 2016

Contributor

That is weird, all tests for this pass in my local branch.

Contributor

mcmillhj commented Feb 12, 2016

That is weird, all tests for this pass in my local branch.

my $weight = 0;
foreach my $product ( grep { defined $_->weight } $self->products_array ) {
$weight += $product->weight;

This comment has been minimized.

@SysPete

SysPete Feb 12, 2016

Member

Looks like you dropped quantity - this works:
$weight += $product->weight * $product->quantity;

@SysPete

SysPete Feb 12, 2016

Member

Looks like you dropped quantity - this works:
$weight += $product->weight * $product->quantity;

Merge branch 'feature/optionally-dont-combine-products-by-sku'
Conflicts:
	lib/Interchange6/Cart/Product.pm
@SysPete

This comment has been minimized.

Show comment
Hide comment
@SysPete

SysPete Feb 12, 2016

Member

@mcmillhj Any chance you could remove the nasty merge commit of master and just rebase once you've updated the code? Don't worry if that is a problem as I can always cherry-pick commits from your branch.

Cheers!

Member

SysPete commented Feb 12, 2016

@mcmillhj Any chance you could remove the nasty merge commit of master and just rebase once you've updated the code? Don't worry if that is a problem as I can always cherry-pick commits from your branch.

Cheers!

@mcmillhj

This comment has been minimized.

Show comment
Hide comment
@mcmillhj

mcmillhj Feb 12, 2016

Contributor

Sure, to do that I just need to:

git checkout <branch>
git rebase upstream/master

right?

Contributor

mcmillhj commented Feb 12, 2016

Sure, to do that I just need to:

git checkout <branch>
git rebase upstream/master

right?

@mcmillhj

This comment has been minimized.

Show comment
Hide comment
@mcmillhj

mcmillhj Feb 12, 2016

Contributor

Hmm, not sure what I have done here. I can always delete this branch and re-created since the changes are minimal.

Contributor

mcmillhj commented Feb 12, 2016

Hmm, not sure what I have done here. I can always delete this branch and re-created since the changes are minimal.

@SysPete

This comment has been minimized.

Show comment
Hide comment
@SysPete

SysPete Feb 12, 2016

Member

Looking at your current enhancement/no-map-in-void-context branch I'd be doing something along the lines of...

git checkout master
git pull upstream master # if you have problems here then merge --abort and reset --hard a few commits and try again
git checkout enhancement/no-map-in-void-context
git reset --hard bbf8f065efa213552d553d6f805d3c01ee584c8d # junk the merge commit
git rebase master
Member

SysPete commented Feb 12, 2016

Looking at your current enhancement/no-map-in-void-context branch I'd be doing something along the lines of...

git checkout master
git pull upstream master # if you have problems here then merge --abort and reset --hard a few commits and try again
git checkout enhancement/no-map-in-void-context
git reset --hard bbf8f065efa213552d553d6f805d3c01ee584c8d # junk the merge commit
git rebase master
@SysPete

This comment has been minimized.

Show comment
Hide comment
@SysPete

SysPete Feb 12, 2016

Member

And I see travis is catching you out with the new test file missing from MANIFEST. Happens to me all the time too which is why we run RELEASE_TESTING there. Just wish I could always remember to run release tests before I push. 😄

Member

SysPete commented Feb 12, 2016

And I see travis is catching you out with the new test file missing from MANIFEST. Happens to me all the time too which is why we run RELEASE_TESTING there. Just wish I could always remember to run release tests before I push. 😄

@mcmillhj

This comment has been minimized.

Show comment
Hide comment
@mcmillhj

mcmillhj Feb 12, 2016

Contributor

Hmm, I just realized that there is another branch merged into these changes that is actually for a different issue. Sorry, not super used to submitting multiple PRs at a time for the same repo. I need to revert the merge of bf4ae52

Contributor

mcmillhj commented Feb 12, 2016

Hmm, I just realized that there is another branch merged into these changes that is actually for a different issue. Sorry, not super used to submitting multiple PRs at a time for the same repo. I need to revert the merge of bf4ae52

@SysPete

This comment has been minimized.

Show comment
Hide comment
@SysPete

SysPete Feb 12, 2016

Member

No worries. I understand completely as I spend most of my time pushing directly to master in most projects. Only when I'm working on something like Dancer2 do I need to remember to branch and keep my PRs clean and separated. Messed up so many times when I first started working like that.

Member

SysPete commented Feb 12, 2016

No worries. I understand completely as I spend most of my time pushing directly to master in most projects. Only when I'm working on something like Dancer2 do I need to remember to branch and keep my PRs clean and separated. Messed up so many times when I first started working like that.

@mcmillhj mcmillhj closed this Feb 12, 2016

@mcmillhj mcmillhj deleted the mcmillhj:enhancement/no-map-in-void-context branch Feb 12, 2016

@mcmillhj

This comment has been minimized.

Show comment
Hide comment
@mcmillhj

mcmillhj Feb 12, 2016

Contributor

Starting fresh.

Contributor

mcmillhj commented Feb 12, 2016

Starting fresh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment