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

revmoving multiples rows with function was failing, when detected indice... #210

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

Conversation

alexmasselot
Copy link

Indices of lines to be removed are stored increasingly.

reversing the list of index solved it

Alexandre Masselot added 3 commits September 20, 2013 18:08
…ices to be removed are stored increasingly.

reversing the list of index solved it
…. The fix at the _add colum should certainly happen better at the _sync level (but how to go from d.changed keeping only the column of interest???)
@iros
Copy link
Member

iros commented Oct 1, 2013

Can we separate teh #207 issue from the reversing of rows? I agree that both are issues but I don't want to bundle them in the same pull request. I reviewed the fix for #207 and that looks good. Not sure about the reversing quite yet.

@alexmasselot
Copy link
Author

Sure! I'm not used to github and somehow it merged the two pull request.
On Oct 1, 2013 3:07 PM, "Irene Ros" notifications@github.com wrote:

Can we separate teh #207 issue from the reversing of rows? I agree that
both are issues but I don't want to bundle them in the same pull request. I
reviewed the fix for #207 and that looks good. Not sure about the reversing
quite yet.


Reply to this email directly or view it on GitHub.

@alexmasselot
Copy link
Author

All this makes sense. Changed made and pushed

123.23e-3 was not passing as number
@jalada
Copy link

jalada commented Jul 14, 2014

It seems really unnecessary to reverse the entire array for iterating. Why not just unshift instead of push so that the array is the right way round?

@alexmasselot
Copy link
Author

Hi David,

Thanks for your email

By now way I claim that to be the best solution. I was stucked there and
found the simplest way around.
In the other hand, I put a few unit test there for that matter, so try any
solution that fit them. Isn't that the goal:)

happy coding
Alex

On Mon, Jul 14, 2014 at 6:52 AM, David Somers notifications@github.com
wrote:

It seems really unnecessary to reverse the entire array for iterating. Why
not just unshift instead of push so that the array is the right way round?


Reply to this email directly or view it on GitHub
#210 (comment).

Alexandre Masselot

http://alexandre-masselot.blogspot.com/
http://mtroopgoeswest.blogspot.com/

@jalada
Copy link

jalada commented Jul 16, 2014

Indeed, I submitted another pull request (#223) Alex :)

@alexmasselot
Copy link
Author

super cool!
Alex

On Wed, Jul 16, 2014 at 6:22 AM, David Somers notifications@github.com
wrote:

Indeed, I submitted another pull request (#223
#223) Alex :)


Reply to this email directly or view it on GitHub
#210 (comment).

Alexandre Masselot

http://alexandre-masselot.blogspot.com/
http://mtroopgoeswest.blogspot.com/

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.

None yet

3 participants