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

Credits for Rob Tovey #98

Merged
merged 4 commits into from
Aug 27, 2020

Conversation

robtovey
Copy link
Contributor

Pull request to address issue #97.

Original commits in #33 did not log correct details for contribution.
Reverted to original branch then moved forward to final merging commit to link with my account.

Hopefully now it will be easier for people to complain when they find all my bugs ;)

@pc494
Copy link
Member

pc494 commented Jul 21, 2020

Great, thanks for this, I'll get to work on the merge conflicts :)

EDIT: turns out this is slightly more involved than originally anticipated.

@pc494 pc494 self-assigned this Jul 21, 2020
@dnjohnstone
Copy link
Member

I know this is non-ideal - but I do think that we should just accept that a mistake was made and move on, it seems like a lot of effort to resolve for not a lot of benefit?

@dnjohnstone dnjohnstone added this to the v0.3.0 milestone Jul 21, 2020
@robtovey
Copy link
Contributor Author

I had another think about this last night and I don't actually think it will work anyway.

Any changes which I originally made and are still in the master wont conflict so they wont change so they wont gain my reference.
When you go through and solve the conflicts with the master you'll just keep everything the same so similarly, none of the attributions will change on the master.

Overall, this is probably a lot of work for absolutely no change at all...

@robtovey
Copy link
Contributor Author

robtovey commented Aug 3, 2020

Sorry, do you think we can confirm which way this is heading?
I don't think this merge will actually do what we were aiming for and I can't think of anything that will.

I'm sorry I made a bit of a mess there but maybe the best thing to do is just let this slide into history...?

@pc494
Copy link
Member

pc494 commented Aug 3, 2020

I think I could make this work, but I've yet to get around to it, if the thing I was planning doesn't get it there I'll give up and, like you say, let it slide into history (if you could leave this PR open that would help though! :) )

@robtovey
Copy link
Contributor Author

robtovey commented Aug 4, 2020

That's fine with me. I just hope you don't waste too much time on it.
Feel free to close the PR and issue whenever you want without checking with me first but also ask if there's anything I can help with on my end.

@dnjohnstone dnjohnstone linked an issue Aug 20, 2020 that may be closed by this pull request
pc494 added a commit to pc494/diffsims that referenced this pull request Aug 27, 2020
pc494 added a commit to pc494/diffsims that referenced this pull request Aug 27, 2020
pc494 added a commit to pc494/diffsims that referenced this pull request Aug 27, 2020
@dnjohnstone dnjohnstone merged commit 7ce9c92 into pyxem:master Aug 27, 2020
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.

Credits for Rob Tovey
3 participants