search

pola-rs / r-polars

Bring polars to R
https://pola-rs.github.io/r-polars/
Other
405 stars 35 forks source link

Revert commits pushed to the main branch by accident #237

Closed eitsupi closed 11 months ago

eitsupi commented 11 months ago

See https://github.com/pola-rs/r-polars/commit/08ce8c45c2a903e375ae0d344e0065e36ee4f2b0#r117064940

etiennebacher commented 11 months ago

the problem is that melt and some others were already merged in main but I forgot to update my main before merging which is why I had to fix the conflict. Since the CI was fine with my 2 commits, can't we just let them there?

eitsupi commented 11 months ago

the problem is that melt and some others were already merged in main but I forgot to update my main before merging which is why I had to fix the conflict. Since the CI was fine with my 2 commits, can't we just let them there?

I cannot determine what has been changed because at first glance I do not know what has been changed

I am fine either way, but I believe that submitting it again as a Pull Request is the more reliable way. If we follow the changes later, we will not really know what happened.

sorhawell commented 11 months ago

That resolve conflicts have tricked me before also.

Ok so we remove option to override in branchrules. And I guess if any admin really really need to override some day, then admin must temporaily toggle the branch rule.

etiennebacher commented 11 months ago

I will redo a clean PR with these changes, it's not too big anyway

etiennebacher commented 11 months ago

But @eitsupi I think reverting this goes back too early, it was already 0.6.1 before my first commit.

eitsupi commented 11 months ago

But @eitsupi I think reverting this goes back too early, it was already 0.6.1 before my first commit.

@etiennebacher I'm sorry but I don't understand what you are talking about. It would be very helpful if you could do this.

sorhawell commented 11 months ago

@etiennebacher just point to a PR which you think is fair. Then we can all review it and add any changes. If not exactly the same so be it.

eitsupi commented 11 months ago

Would it be better to force push to the main branch?

etiennebacher commented 11 months ago

Thank you @eitsupi I think the last force-push solved it, this PR now goes back to the state before my last 2 commits on main. I think this can be merged and I will make a new PR with my 2 commits

eitsupi commented 11 months ago

Ok, I finally could back to 0.6.1 with this command:

git diff 08ce8c45c2a903e375ae0d344e0065e36ee4f2b0..ddf5510b47ada712ff7838cac74bfbf3f5a226dd >to_fix.diff
mv to_fix.diff /tmp/to_fix.diff
git apply /tmp/to_fix.diff

4b897bcd67a36f831b4707fac7a629789c72ddb8 is completely identical to 0.6.1

git diff 4b897bcd67a36f831b4707fac7a629789c72ddb8..ddf5510b47ada712ff7838cac74bfbf3f5a226dd
etiennebacher commented 11 months ago

FYI I think a simpler solution would have been git reset ddf5510b47ada712ff7838cac74bfbf3f5a226dd (last "good" commit)

eitsupi commented 11 months ago

Thanks @etiennebacher, It would be sufficient to simply create a PR on the branch where you revert the commit that merged this.

eitsupi commented 11 months ago

FYI I think a simpler solution would have been git reset ddf5510b47ada712ff7838cac74bfbf3f5a226dd (last "good" commit)

My understanding is that git reset only moves the tree and does not create a new commit.