{"action":"created","issue":{"url":"https://api.github.com/repos/appium/appium/issues/6259","repository_url":"https://api.github.com/repos/appium/appium","labels_url":"https://api.github.com/repos/appium/appium/issues/6259/labels{/name}","comments_url":"https://api.github.com/repos/appium/appium/issues/6259/comments","events_url":"https://api.github.com/repos/appium/appium/issues/6259/events","html_url":"https://github.com/appium/appium/pull/6259","id":139637529,"number":6259,"title":"Remove npm-shrinkwrap from master branch","user":{"login":"imurchie","id":2614354,"avatar_url":"https://avatars.githubusercontent.com/u/2614354?v=3","gravatar_id":"","url":"https://api.github.com/users/imurchie","html_url":"https://github.com/imurchie","followers_url":"https://api.github.com/users/imurchie/followers","following_url":"https://api.github.com/users/imurchie/following{/other_user}","gists_url":"https://api.github.com/users/imurchie/gists{/gist_id}","starred_url":"https://api.github.com/users/imurchie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/imurchie/subscriptions","organizations_url":"https://api.github.com/users/imurchie/orgs","repos_url":"https://api.github.com/users/imurchie/repos","events_url":"https://api.github.com/users/imurchie/events{/privacy}","received_events_url":"https://api.github.com/users/imurchie/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2016-03-09T16:58:49Z","updated_at":"2016-03-09T17:48:06Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/appium/appium/pulls/6259","html_url":"https://github.com/appium/appium/pull/6259","diff_url":"https://github.com/appium/appium/pull/6259.diff","patch_url":"https://github.com/appium/appium/pull/6259.patch"},"body":"## Proposed changes\r\n\r\nI’m beginning to question why we have `npm-shrinkwrap.json` in our master branch. It makes sense to lock the dependencies down in the release branch, but `master` is a de facto development branch, so we want to be able to test out packages as they are updated. Having `npm-shrinkwrap.json` there means that our local branches are always dirty, since we have to delete it to do any work.\r\n\r\nFurther, for contributors trying to do stuff in `master` that involves new dependencies (see, for instance, #6215), it means the burden of dealing with shrinkwrap and making sure things are actually sane.\r\n\r\nRemoving `npm-shrinkwrap.json` in `master` has no consequences for normal users. Release will be locked in their respective branches, and master will remain truly the \"source\" install location.\r\n\r\n## Types of changes\r\n\r\nWhat types of changes does your code introduce to Appium?\r\n_Put an `x` in the boxes that apply_\r\n\r\n- [ ] Bugfix (non-breaking change which fixes an issue)\r\n- [x] New feature (non-breaking change which adds functionality)\r\n- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)\r\n\r\n## Checklist\r\n\r\n_Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._\r\n\r\n- [x] I have read the [CONTRIBUTING](/CONTRIBUTING.md) doc\r\n- [x] I have signed the [CLA](https://docs.google.com/forms/d/1lOfXRw_0VCk7gYzjj4WLetGu7yelDVo5LWh0z7pGftE/viewform)\r\n- [x] Lint and unit tests pass locally with my changes\r\n- [ ] I have added tests that prove my fix is effective or that my feature works\r\n- [ ] I have added necessary documentation (if appropriate)\r\n- [ ] Any dependent changes have been merged and published in downstream modules\r\n\r\n## Further comments\r\n\r\nThere are currently tests to make sure that we have a shrinkwrap file, and that the optional dependencies are removed from it. I've skipped them here, but in general I'm not sure of their utility. On release we run our tests without it, so they will fail anyway. So as long as we follow the release process it should be there. Or perhaps the test ought to be, somehow, tied to the `npm publish` task, so that we cannot publish without it, but we can run the regular test suite.\r\n\r\n### Reviewers: @scottdixon, @jlipps\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/appium/appium/issues/comments/194421531","html_url":"https://github.com/appium/appium/pull/6259#issuecomment-194421531","issue_url":"https://api.github.com/repos/appium/appium/issues/6259","id":194421531,"user":{"login":"jlipps","id":605053,"avatar_url":"https://avatars.githubusercontent.com/u/605053?v=3","gravatar_id":"","url":"https://api.github.com/users/jlipps","html_url":"https://github.com/jlipps","followers_url":"https://api.github.com/users/jlipps/followers","following_url":"https://api.github.com/users/jlipps/following{/other_user}","gists_url":"https://api.github.com/users/jlipps/gists{/gist_id}","starred_url":"https://api.github.com/users/jlipps/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jlipps/subscriptions","organizations_url":"https://api.github.com/users/jlipps/orgs","repos_url":"https://api.github.com/users/jlipps/repos","events_url":"https://api.github.com/users/jlipps/events{/privacy}","received_events_url":"https://api.github.com/users/jlipps/received_events","type":"User","site_admin":false},"created_at":"2016-03-09T17:48:06Z","updated_at":"2016-03-09T17:48:06Z","body":"I like this. However I'd like to make sure that the developers-overview.md is updated if necessary to clarify why this is would only be on the release branch, and instructions about re-shrinkwrapping and pushing that change _only_ on the release branch"}}