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

allow multiple organize calls in an organizer #134

Conversation

hedgesky
Copy link
Contributor

Implements #127.

Copy link
Collaborator

@laserlemon laserlemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good, thank you!

# end
#
# Returns nothing.
def organize(*interactors)
@organized = interactors.flatten
@organized ||= []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This already happens on line 66/68.

# end
#
# Returns nothing.
def organize(*interactors)
@organized = interactors.flatten
@organized ||= []
@organized += interactors.flatten
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than using +=, we could concat which won't duplicate the array in memory.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def organize(*interactors)
  organized.concat(interactors.flatten)
end

@hedgesky hedgesky force-pushed the allow_multiple_organize_calls_in_an_organizer branch from 92656c0 to d9de996 Compare March 26, 2017 14:20
@hedgesky
Copy link
Contributor Author

I've made the following change:

-        @organized ||= []
-        @organized += interactors.flatten
+        organized.concat(interactors.flatten)

Fixed up already existing commit.

@hedgesky hedgesky force-pushed the allow_multiple_organize_calls_in_an_organizer branch from d9de996 to aaa2bb6 Compare March 26, 2017 14:31
@laserlemon
Copy link
Collaborator

@hedgesky I made a mess of this trying to resolve conflicts in the web interface. Could you please rebase this against v4 or cherry-pick your one commit against a new branch off of v4 and force push? Something to dig me out of this. Thanks!

@hedgesky hedgesky force-pushed the allow_multiple_organize_calls_in_an_organizer branch from 675b049 to 5d28fbe Compare March 31, 2017 20:36
@hedgesky
Copy link
Contributor Author

Done.

@laserlemon laserlemon merged commit 15e70dd into collectiveidea:v4 Mar 31, 2017
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