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

Add missing member variable social_media_image_file_ID, fix formatting #69

Conversation

Projects
None yet
2 participants
@davidnewcomb
Copy link
Contributor

commented Mar 8, 2018

Chapter::social_media_image_file_ID is missing from the class definition so PHP blows up if it is strict.
There were extra spaces at the start of some of the lines before the tabs so I removed them.

@fplanque
Copy link
Contributor

left a comment

ok

@fplanque
Copy link
Contributor

left a comment

@davidnewcomb This should not be merged to the master branch but to the appropriate release/6.9.x or to develop. I don't know how to change this in the pull request. Can it be done?

@fplanque fplanque added invalid Wrong branch and removed invalid labels Apr 5, 2018

davidnewcomb added some commits Mar 8, 2018

Rebase to develop
Merge branch 'Undefined_property_Chapter_social_media_image_file_ID' of https://github.com/davidnewcomb/b2evolution into Undefined_property_Chapter_social_media_image_file_ID
@davidnewcomb

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2018

It seems the @winskie fixed it on develop on 9 Dec 17.
7aed12e

This is one of my first push's to the project. If I had known that develop was the right branch then I would have seen the change and not submitted a patch.

If only there was a place where all that useful developer information was written down ;)

In answer to your question yes it is possible. git rebase is your friend. It winds back all the commits on your feature branch, creates a new branch at the end of the branch you want, then reapplies your commits to the new branch asking you to resolve conflicts after each commit. Think of it as "I meant to start here"
https://hackernoon.com/git-merge-vs-rebase-whats-the-diff-76413c117333

I have rebased Undefined_property_Chapter_social_media_image_file_ID to develop, so now the merge is trivial.
My change has been reduced to removing those extra spaces I mentioned in my initial pull request :(
But they are still valid changes!

@fplanque

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2018

@davidnewcomb ok, thanks for explaining rebase... I think I understand the concept, but I still don't understand how it applies to a pull request like this. This pull request still says it wants to merge to b2evolution:master and if I press the green button, I'll get a huge mess of 429 files changed ina branch that we don't want to change.

@davidnewcomb davidnewcomb changed the base branch from master to develop Apr 5, 2018

@davidnewcomb

This comment has been minimized.

Copy link
Contributor Author

commented Apr 6, 2018

When I made the pull request, I did it from master because I didn't know any better. I've changed this pull request to merge to develop instead (same as rebased code now). I'm not completely sure why there are extra commits in there but it says that they have already been merged so maybe it's just a side effect of something.

Now that the bug has been fixed It's only a few spaces in a file and it's not really worth all this extra hassle. I think those extra commits will be no-ops but I'm not sure and I don't want to mess anything up.

It would probably be easier and cleaner for me to do the work again on a clean branch. Thinking about it, as it's just a formatting bug, I can fix it on my spelling branch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.