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

merge master on drop_py27 #69

Merged
merged 3 commits into from
May 28, 2021
Merged

merge master on drop_py27 #69

merged 3 commits into from
May 28, 2021

Conversation

kontsaki
Copy link
Contributor

this fixes the conflicts i've introduced by formatting with black

@goodboy
Copy link
Member

goodboy commented Feb 13, 2021

@kontsaki the problem now is this reverts changes from the target branch (such as the email change) among other things.

Merging back from master like this just basically takes all the conflicts wholesale from master instead of actually resolving them.

This is why I said the rebase needs to be done - merging branches back like this also leave a pretty ugly looking git history.

@kontsaki
Copy link
Contributor Author

the problem now is this reverts changes from the target branch (such as the email change) among other things.

@goodboy do you mean that there are changes that are missing?

sorry, i haven't used rebase much to understand what's the difference with merging except merge commits

@kontsaki
Copy link
Contributor Author

i'm reading that rebasing is not good for already published branches, it will require a force push if i understand correctly

@goodboy
Copy link
Member

goodboy commented Mar 21, 2021

i'm reading that rebasing is not good for already published branches, it will require a force push if i understand correctly

Yes nothing wrong with this imo. IIRC there was something distinctly missing last time I looked at this diff; it's the cognative load of looking through so many non-functional changes that makes me put this kinda thing off 😉

I remember at the least the new email in setup.py looks like it was lost, there might be more?

I have to go through this in detail I think unfortunately...

Again this is why I'd recommend in the future making functional changes separate from broad style patches.

@kontsaki
Copy link
Contributor Author

kontsaki commented Mar 22, 2021

setup.py email is the same at master and #55

same for __init__.py even though they differ from setup.py
this seems to be an old mismatch

@goodboy
Copy link
Member

goodboy commented Mar 22, 2021

Ok we'll do it your way and try merging this because what I thought was off isn't 😂

I'm going to push up a clone of the branch for #63 to see how things look after merging this into that branch.

@kontsaki
Copy link
Contributor Author

ping :)

@goodboy
Copy link
Member

goodboy commented May 28, 2021

Aight pushed up a clone of drop_py27 in case we run into any issues 🕶️.

Gonna merge this!

@goodboy goodboy merged commit acf8374 into SIPp:drop_py27 May 28, 2021
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