Tally margins

[carlschroedl]

This is a first pass at addressing #7.

[coveralls]

Coverage decreased (-57.6%) to 42.446% when pulling a9cf00455a958b325cb93bc2989b0a3a9093a5f4 on carlschroedl:tallyMargins#7 into eabd5ce03bba9d52b09953e24d74cfa675020ebd on pivot-libre:0.x.

[carlschroedl]

Sorry for the extra commits. I have no idea why these superfluous commits are showing up on Github. When I run this command locally to list which commits in the feature branch are not part of 0.x, I get a much shorter list of commits:

$> git log 0.x..tallyMargins#7
a9cf00455a958b325cb93bc2989b0a3a9093a5f4 Improving NBallot documentation.
cbeb01a47127d273052b082372a6d42455b52b95 Added test for setMargin method of Margin class.
64ed85d83b2a3a83a36ca7a785f315e454af2fad Switched Agenda to use an array instead of an SplObjectStorage. Implemented tests.
0708dae96172fd8e54533498b629702728226ac9 First pass implementing the MarginCalculator's getCandidateIdToRankMap method.
8fa78bbfde4e00935719e084ee50401084e51850 Improved documentation and formatting on the Agenda class.
9eebdd098edb2406bcfe20504901bb885940c5fd Added skeleton test class for MarginCalculator.
24acd393e9d6f536358eaa1e129df337cac5255d First pass at Agenda class methods.
9b846ce597b3cd2d24f57c950b06904a919a67a9 Converted some margin calculation psuedocode to code.
9acb01dfda071c0e0c392ac1cf2312539866fa1a Added LOUD reminder
d24f238e9df8b901ae75fff2aa675c808a2f89a5 Fixed php cs errors
f89df0b91149f536564b8b45cd205fed47336c22 TEst class.
8d5d3fea67d4246cfc29114e24e74b4b2da300c9 Broken checkpoint
b4b614a4e496014d593502a39819fd2a4d2d8e94 Improved test coverage on the MarginRegistry.
68ecfb61fbe573c4646718928dfecefddcec4ddc New classes and tests for margin calculation.
b88317a0765f6ec2bc02a84bd9fd108c209076bc Renamed Majority to Margin.
97e76ea34c91f393fc73178f079092697639bf5e Removed .gitkeep file that is no longer needed.
97f097e66b2c3c72a749a45cf1e3ce8c699cc43b Adding new subclass of Ballot to enable easier testing of margin computation.

This branch was created by branching off of 0.x. Before submitting the PR, I checked out 0.x, and pulled from upstream 0.x. Then I checked out the feature branch and rebased on 0.x. Then I force pushed the feature branch to my fork, and created the PR.

Any idea where I'm going wrong here?

[coveralls]

Coverage decreased (-57.6%) to 42.446% when pulling c69ad8b0d23630dc434713deac198c828f56596a on carlschroedl:tallyMargins#7 into eabd5ce03bba9d52b09953e24d74cfa675020ebd on pivot-libre:0.x.

[coveralls]

Coverage decreased (-60.4%) to 39.597% when pulling 3501d559056936d53ecaa7a0acafc0595ba31dce on carlschroedl:tallyMargins#7 into eabd5ce03bba9d52b09953e24d74cfa675020ebd on pivot-libre:0.x.

[coveralls]

Coverage decreased (-35.3%) to 64.706% when pulling 556ba0c67be87729bf88c761f4849218e6f98e3b on carlschroedl:tallyMargins#7 into eabd5ce03bba9d52b09953e24d74cfa675020ebd on pivot-libre:0.x.

[coveralls]

Coverage decreased (-25.2%) to 74.766% when pulling 5377d3cc15d25b91e2b429847e746c7fdd824ce5 on carlschroedl:tallyMargins#7 into eabd5ce03bba9d52b09953e24d74cfa675020ebd on pivot-libre:0.x.

[coveralls]

Coverage decreased (-21.5%) to 78.505% when pulling db7438a9a3a240469861d5b715d90fbe971e1039 on carlschroedl:tallyMargins#7 into eabd5ce03bba9d52b09953e24d74cfa675020ebd on pivot-libre:0.x.

[coveralls]

Coverage decreased (-15.6%) to 84.404% when pulling fe80050ff2735c0d5898e3f94f2888f54d70a461 on carlschroedl:tallyMargins#7 into eabd5ce03bba9d52b09953e24d74cfa675020ebd on pivot-libre:0.x.

[coveralls]

Coverage remained the same at 100.0% when pulling f033005400676bdde98d08f6fc69fff497a02f0f on carlschroedl:tallyMargins#7 into eabd5ce03bba9d52b09953e24d74cfa675020ebd on pivot-libre:0.x.

[coveralls]

Coverage remained the same at 100.0% when pulling 037628123875aea162d89253c588736686140c2c on carlschroedl:tallyMargins#7 into 0138db00d9a8d11af132f9834ceea84e68b7621b on pivot-libre:0.x.

[andrewshell]

This was very hard to merge because you kept merging 0.x into your branch. That causes a lot of duplicates and collisions. Either rebase from 0.x to keep your branch up to date or just don't try to pull in 0.x at all. Merges just gum everything up.

I was finally able to clean this up and resubmit as #35 which has been merged.

If there are more than 3-4 commits in a PR something is wrong. Keep them small and frequent so we can stay on top of what's going on.

If you have lots of little commits please squash them before creating the PR.

[carlschroedl]

Thanks for cleaning it up. I'm sure that made you a bit mad. Please understand that it was not intentional.

Please keep in mind:

1. Rebasing is still foreign to me
2. I'm not convinced of the value of this workflow
3. I'm trying it out anyway on faith that it is worthwhile

Could you please directly address the question I left at the very beginning of this pull request's conversation? I would like to know where I was going wrong in the workflow so that I can improve.

I agree that feature branches and PR's should generally not be so long-running. I think this case was different. In this case, I issued a pull request early because I was hoping to get feedback on approaches. No one had time to review, so I experimented, committing the results of ~30min short bursts over many days which eventually evolved into something workable. I think it is very important that the code in this library be reviewed before merging, so I did not self-merge.

[carlschroedl]

Ah git pull = git fetch && git merge So is the preferred way in this workflow git fetch and git rebase?