{"action":"created","issue":{"url":"https://api.github.com/repos/gemini-hlsw/ocs/issues/194","labels_url":"https://api.github.com/repos/gemini-hlsw/ocs/issues/194/labels{/name}","comments_url":"https://api.github.com/repos/gemini-hlsw/ocs/issues/194/comments","events_url":"https://api.github.com/repos/gemini-hlsw/ocs/issues/194/events","html_url":"https://github.com/gemini-hlsw/ocs/pull/194","id":53888438,"number":194,"title":"OCSADV-200: work-in-progress on diff-based merge","user":{"login":"swalker2m","id":4906023,"avatar_url":"https://avatars.githubusercontent.com/u/4906023?v=3","gravatar_id":"","url":"https://api.github.com/users/swalker2m","html_url":"https://github.com/swalker2m","followers_url":"https://api.github.com/users/swalker2m/followers","following_url":"https://api.github.com/users/swalker2m/following{/other_user}","gists_url":"https://api.github.com/users/swalker2m/gists{/gist_id}","starred_url":"https://api.github.com/users/swalker2m/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/swalker2m/subscriptions","organizations_url":"https://api.github.com/users/swalker2m/orgs","repos_url":"https://api.github.com/users/swalker2m/repos","events_url":"https://api.github.com/users/swalker2m/events{/privacy}","received_events_url":"https://api.github.com/users/swalker2m/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-01-09T16:50:58Z","updated_at":"2015-01-09T20:57:20Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/gemini-hlsw/ocs/pulls/194","html_url":"https://github.com/gemini-hlsw/ocs/pull/194","diff_url":"https://github.com/gemini-hlsw/ocs/pull/194.diff","patch_url":"https://github.com/gemini-hlsw/ocs/pull/194.patch"},"body":"This PR updates the difference calculation that will be employed by the sync/merge algorithm.  In particular, it:\r\n\r\n* moves new code into a temporary package `edu.gemini.sp.vcs.diff` to help me separate classes involved in the old algorithm in `edu.gemini.sp.vcs` from the new.  When the new algorithm is done, the old code will be removed and the content of this package can be moved back into `edu.gemini.sp.vcs`.\r\n* simplifies and flattens difference nodes into just two varieties: those present in the local program vs. those that are missing.  That is, the previous `InUse` turned into `Present` and the old `Missing` and `Removed` were merged into just `Missing`.\r\n* updates the comparison to take into account whether a node is present in the remote program.  Even when a node’s `VersionVector` is the same across the local and remote program, we still want to know about it if the node is missing in one instance of the program.\r\n* implements `scalaz.Equal` instances for `VersionVector`.\r\n\r\nTesting feels inadequate.  I could use some pointers for how/whether to turn this into property based tests checked by ScalaCheck, presumably using a Generator that produces program edit functions."},"comment":{"url":"https://api.github.com/repos/gemini-hlsw/ocs/issues/comments/69397732","html_url":"https://github.com/gemini-hlsw/ocs/pull/194#issuecomment-69397732","issue_url":"https://api.github.com/repos/gemini-hlsw/ocs/issues/194","id":69397732,"user":{"login":"tpolecat","id":1200131,"avatar_url":"https://avatars.githubusercontent.com/u/1200131?v=3","gravatar_id":"","url":"https://api.github.com/users/tpolecat","html_url":"https://github.com/tpolecat","followers_url":"https://api.github.com/users/tpolecat/followers","following_url":"https://api.github.com/users/tpolecat/following{/other_user}","gists_url":"https://api.github.com/users/tpolecat/gists{/gist_id}","starred_url":"https://api.github.com/users/tpolecat/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tpolecat/subscriptions","organizations_url":"https://api.github.com/users/tpolecat/orgs","repos_url":"https://api.github.com/users/tpolecat/repos","events_url":"https://api.github.com/users/tpolecat/events{/privacy}","received_events_url":"https://api.github.com/users/tpolecat/received_events","type":"User","site_admin":false},"created_at":"2015-01-09T20:57:20Z","updated_at":"2015-01-09T20:57:20Z","body":"This all looks good to me. :+1: \r\n\r\nAs for property-based testing, you can state some invariants. For instance:\r\n\r\n- Basic stuff; if a given node differs then it must appear in the diff list, etc.\r\n- If `a` appears in the diff list and `b` is an ancestor of `a`, then `b` must also appear in the diff list.\r\n- Any pair of nodes that share the same ancestor observation must both appear in the diff list, or both not appear in the diff list.\r\n\r\nSo you can maybe start with a simple constant program and apply random changes to a pair of them and ensure that the invariants are always met. I'm not sure how helpful this will be in the end but it might help detect regressions in the end, and might be useful for handling more complex invariants that have to deal with program status and so on.\r\n"}}