Added includeDeepMembers

[qbolec]

As requested in #572 (comment) I've added the includeDeepMembers function for symmetry.

I have several doubts though:

• is it ok that includeDeepMembers and includeMembers ignore duplicates in the subset argument? I'd expect the function to ignore order, but not quantity of elements to be a useful feature for some scenarios (same question for sameDeepMembers, and sameMembers).
• I've added a testcase to explicitly document current behaviour described above. Is it ok to add such tests or is it unnecessarily constraining future modifications of implementation?
• why is generated chai.js so much different from the one in head? Is it ok to commit this file anyway? should it even be in the repo?

[IanVS]

Hi @qbolec, thanks for tackling this. Based on the message "This branch is out-of-date with the base branch", it looks like perhaps you made the changes on a local copy of your repo which is not up-to-date with master. Here's what I suggest you do.

If you haven't added this main repo as an upstream remote:

git remote add upstream https://github.com/chaijs/chai.git

Then, rebase your local master onto the upstream master:

git rebase upstream/master

This may fail with merge conflicts. You will need to open the files which have conflicts, edit them to look like the way you want (making sure to remove the >>> type symbols which git adds), save and stage them, then use git rebase --continue to continue the rebase. After this is all done, you can force push up to your remote repo with:

git push -f

For dealing with merge conflicts, if you use Atom I would suggest checking out (merge-conflicts)[https://github.com/smashwilson/merge-conflicts], which adds a nice UI around fixing merge conflicts.

If you get stuck or have questions, let us know. Thanks again for your work here.

[IanVS]

Another suggestion I have is to use feature branches, to avoid this kind of trouble in the future.

git checkout master
git pull upstream master
git push # Not strictly necessary, but will keep your remote master up-to-date
git checkout -b <name-of-your-feature-or-issue-number>

This will create a new branch, starting with the most up-to-date code from this project's master branch. Then, you can work on multiple features or issues at a time, while keeping your master branch in sync with the upstream.

[keithamus]

Thanks for the PR @qbolec! @IanVS gives some good advice, and also I have a little bit more for you:

I'd kindly ask you to remove chai.js from your commit - as we compile this once per release, rather than with every commit. To remove it, you should just need to run this commands:

git rebase -i origin/master
# This will bring up an editor, which will say:
#  `pick 9b8fe76 Added includeDeepMembers`.
# Change this to:
# `edit 9b8fe76 Added includeDeepMembers`. 
git rm chai.js
git commit --amend
# Editor again, just save and quit
git checkout -b add-includedeepmembers
git push origin add-includedeepmembers

You can read more about rebasing here: https://help.github.com/articles/about-git-rebase/

[qbolec]

Thanks @keithamus and @IanVS for all your advice. My experience with svn seems to be more of a burden than a help with git. I'm still having problems with building a mental model of this whole thing :)
And this currently prevents me from figuring out how to stitch the three pieces of advice I've got into a one coherent plan of action. I'll try to experiment, and if I fail, I think I'll simply start again from scratch with a feature branch :)

So far I tried git remote add upstream https://github.com/chaijs/chai.git followed by git rebase upstream/master which failed with

fatal: Needed a single revision
invalid upstream upstream/master

I've tried to do git pull upstream master instead, which seemed to perform some actions, then git push after which this website immediately updated showing all checks are green now, but frankly I have no idea why :)

[qbolec]

I also tried git rebase -i origin/master but it shows just a single action noop.

[qbolec]

git rebase -i upstream/master seemed to something more similar to what was described by @keithamus. I later did

git rm chai.js
git commit --amend
git rebase --continue
git checkout -b add-includedeepmembers
git push origin add-includedeepmembers

which created some a new branch, for which the "compare changes" shows a rather disturbing removal of chai.js.

I think I'll start over with a fresh repo :)

[keithamus]

git remote is a shortcut that, under the hood, simply edits your .git/config file - it doesn't actually fetch any resources. You need to tell it to fetch your newly added remote by running git fetch upstream (or git fetch --all to fetch all of your remotes at once).

So if you run:

git remote add upstream https://github.com/chaijs/chai.git
git fetch --all
git rebase upstream/master

Then you should see stuff happen.

FYI, "remotes" are really just urls that are listed in your .git/config, with convenient names like "origin" and "upstream". If you cat .git/config you can see everything that drives remotes - you'll see something like

[remote "origin"]
  url = ...

Running git fetch origin or git fetch upstream just downloads the contents from that url. Running git fetch --all will go through every remote you have in your .git/config and download from that url.

rebase is a way of reapplying your changes onto a different branch - the reason you're getting a noop for git rebase origin/master is because origin/master is your current branch (or, more precisely, the HEADs match - but thats a semantic detail). The point is - git rebase will only reapply your new commits onto a differing branch.

[keithamus]

which created some a new branch, for which the "compare changes" shows a rather disturbing removal of chai.js.

Sorry @qbolec! That was because I gave you bad advice! What I should have written was not git rm chai.js but git checkout upstream/master chai.js. This will reset chai.js to the same on that is in upstream/master (provided you have an upstream remote)

[qbolec]

Well, there is nothing to be sorry about :)
Now it even makes sense to my svn-flawed mind!
I'll try again, until I succeed :)

[qbolec]

It seems that performning

git checkout upstream/master chai.js
git commit -m "restore chai.js"
git push

while being switched to add-includedeepmembers fixed the issue, that is now the "compare changes" shows only modifications in the two interesting files. Great.

This is however achieved using two commits, and I've read somewhere that it is a good practice to squash commits before doing PR. Is it necessary? Is it even possible once I've already pushed the two commits to my github remote?

[qbolec]

I've tried

git rebase -i HEAD~2

changed the second line to squash, tired git push origin add-includedeepmembers which failed, so I did git pull origin add-includedeepmembers and tried again git push origin add-includedeepmembers and now I have a branch with 4 commits :)

[IanVS]

You can squash if you like. To do it:

git rebase -i HEAD~2

On the second commit (the one you want to squash into the other), change the pick to s (for squash). Then save, and git will ask you to modify the commit message. Clean that up if you like, then save it as well. After that, you will need to force push to your repository, because you are changing history (blowing away the old commits, and replacing with a new one), so:

git push -f

The pull request should be updated automatically.

[IanVS]

You beat me to it. You're on the right track, just missing the force push. You can use HEAD~4 now to take care of all of them.

[qbolec]

Yep, the missing part was --force.
Instead of --force I tried to use git pull origin add-includedeepmembers, and that only created more mess.
But I think I managed to clean up the mess with:

git rebase -i HEAD~2
git push origin add-includedeepmembers --force

My question now is this: should I create a new PR using the website GUI for the new branch, or should I somehow edit the PR we are currently discussing?
The proposed changeset is this : master...qbolec:add-includedeepmembers

[keithamus]

@qbolec feel free to make a new PR from the add-includedeepmembers branch and I'll close this one 😄