{"action":"created","issue":{"url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556","repository_url":"https://api.github.com/repos/CindyJS/CindyJS","labels_url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556/labels{/name}","comments_url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556/comments","events_url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556/events","html_url":"https://github.com/CindyJS/CindyJS/pull/556","id":198006616,"number":556,"title":"Fix version of js-beautify","user":{"login":"gagern","id":190759,"avatar_url":"https://avatars.githubusercontent.com/u/190759?v=3","gravatar_id":"","url":"https://api.github.com/users/gagern","html_url":"https://github.com/gagern","followers_url":"https://api.github.com/users/gagern/followers","following_url":"https://api.github.com/users/gagern/following{/other_user}","gists_url":"https://api.github.com/users/gagern/gists{/gist_id}","starred_url":"https://api.github.com/users/gagern/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gagern/subscriptions","organizations_url":"https://api.github.com/users/gagern/orgs","repos_url":"https://api.github.com/users/gagern/repos","events_url":"https://api.github.com/users/gagern/events{/privacy}","received_events_url":"https://api.github.com/users/gagern/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"strobelm","id":14185442,"avatar_url":"https://avatars.githubusercontent.com/u/14185442?v=3","gravatar_id":"","url":"https://api.github.com/users/strobelm","html_url":"https://github.com/strobelm","followers_url":"https://api.github.com/users/strobelm/followers","following_url":"https://api.github.com/users/strobelm/following{/other_user}","gists_url":"https://api.github.com/users/strobelm/gists{/gist_id}","starred_url":"https://api.github.com/users/strobelm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/strobelm/subscriptions","organizations_url":"https://api.github.com/users/strobelm/orgs","repos_url":"https://api.github.com/users/strobelm/repos","events_url":"https://api.github.com/users/strobelm/events{/privacy}","received_events_url":"https://api.github.com/users/strobelm/received_events","type":"User","site_admin":false},"assignees":[{"login":"strobelm","id":14185442,"avatar_url":"https://avatars.githubusercontent.com/u/14185442?v=3","gravatar_id":"","url":"https://api.github.com/users/strobelm","html_url":"https://github.com/strobelm","followers_url":"https://api.github.com/users/strobelm/followers","following_url":"https://api.github.com/users/strobelm/following{/other_user}","gists_url":"https://api.github.com/users/strobelm/gists{/gist_id}","starred_url":"https://api.github.com/users/strobelm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/strobelm/subscriptions","organizations_url":"https://api.github.com/users/strobelm/orgs","repos_url":"https://api.github.com/users/strobelm/repos","events_url":"https://api.github.com/users/strobelm/events{/privacy}","received_events_url":"https://api.github.com/users/strobelm/received_events","type":"User","site_admin":false},{"login":"montaga","id":15124152,"avatar_url":"https://avatars.githubusercontent.com/u/15124152?v=3","gravatar_id":"","url":"https://api.github.com/users/montaga","html_url":"https://github.com/montaga","followers_url":"https://api.github.com/users/montaga/followers","following_url":"https://api.github.com/users/montaga/following{/other_user}","gists_url":"https://api.github.com/users/montaga/gists{/gist_id}","starred_url":"https://api.github.com/users/montaga/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/montaga/subscriptions","organizations_url":"https://api.github.com/users/montaga/orgs","repos_url":"https://api.github.com/users/montaga/repos","events_url":"https://api.github.com/users/montaga/events{/privacy}","received_events_url":"https://api.github.com/users/montaga/received_events","type":"User","site_admin":false}],"milestone":null,"comments":4,"created_at":"2016-12-29T13:23:02Z","updated_at":"2016-12-30T10:21:47Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/CindyJS/CindyJS/pulls/556","html_url":"https://github.com/CindyJS/CindyJS/pull/556","diff_url":"https://github.com/CindyJS/CindyJS/pull/556.diff","patch_url":"https://github.com/CindyJS/CindyJS/pull/556.patch"},"body":"Different versions of js-beautify will format our codebase in different ways, even for patch-level version increments.  So I suggest we fix the version the hard way, with the intention of bumping the version every now and then in a conscious update, containing all related code formatting changes in that same commit.\r\n\r\nThis commit does *not* use the latest version at the time of the commit. That would be 1.6.7 which improves [one line of code](https://github.com/CindyJS/CindyJS/blob/8422099f65445e3f67a12ece5c4e22638ca464d0/plugins/cindygl/src/js/CodeBuilder.js#L296) thanks to a fix for https://github.com/beautify-web/js-beautify/issues/1079, but in [other](https://github.com/CindyJS/CindyJS/blob/v0.8.2/src/js/libcs/OpDrawing.js#L325-L329) [places](https://github.com/CindyJS/CindyJS/blob/v0.8.2/src/js/Events.js#L496) it suffers from https://github.com/beautify-web/js-beautify/issues/1090 and https://github.com/beautify-web/js-beautify/issues/1085#issuecomment-269628467 even though I don't know why earlier versions don't affect us there, too.\r\n\r\nI'd like to get this one here in soon, and then hope for the two issues mentioned above to get addressed. And then perhaps @montaga can have a look at bumping to something more recent, as most of the remaining differences are located in the CindyGL codebase."},"comment":{"url":"https://api.github.com/repos/CindyJS/CindyJS/issues/comments/269755234","html_url":"https://github.com/CindyJS/CindyJS/pull/556#issuecomment-269755234","issue_url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556","id":269755234,"user":{"login":"gagern","id":190759,"avatar_url":"https://avatars.githubusercontent.com/u/190759?v=3","gravatar_id":"","url":"https://api.github.com/users/gagern","html_url":"https://github.com/gagern","followers_url":"https://api.github.com/users/gagern/followers","following_url":"https://api.github.com/users/gagern/following{/other_user}","gists_url":"https://api.github.com/users/gagern/gists{/gist_id}","starred_url":"https://api.github.com/users/gagern/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gagern/subscriptions","organizations_url":"https://api.github.com/users/gagern/orgs","repos_url":"https://api.github.com/users/gagern/repos","events_url":"https://api.github.com/users/gagern/events{/privacy}","received_events_url":"https://api.github.com/users/gagern/received_events","type":"User","site_admin":false},"created_at":"2016-12-30T10:21:47Z","updated_at":"2016-12-30T10:21:47Z","body":"@bitwiseman wrote:\r\n> We'd appreciate your help to add tests and make fixes. That will do a better job addressing the problem than simply locking to a specific version.\r\n\r\nWhile I'll be happy to write test cases and file issues whenever js-beautify doesn't do as good a job as I'd expect it to, I have to do so at my own time. When I decide “now I want to bump the js-beautify version”, then I've got the time to carefully look at all the changes and handle them appropriately. But when I'm busy developing something else, then a change in js-beautify behavior, be it for better or worse, is simply a major distraction. As is the case now, where [Travis checks are failing](https://travis-ci.org/CindyJS/CindyJS/jobs/187511599) simply because the latest version of js-beautify doesn't do things exactly the way previous versions did.\r\n\r\nA locked version communicates that “this codebase follows the style of js-beautify version foo”, and that relationship is enforced by our Travis checks. If a more recent version of js-hint formats our code the same way, there is no reason to use the more recent version. If a more recent version formats the code differently, that change in style should be made explicit, and the newer style should be made mandatory for everyone dealing with the codebase from that point onwards. So I still don't see a reason not to lock the version, for our specific use case.\r\n\r\n> You don't have to \"hope for the two issues mentioned above to get addressed\" - you've already done bisects spotted where the change occurred, you're quite a ways toward implementing the tests and fixes yourself.\r\n\r\nI haven't even started looking at how the code actually works. Seeing how people who know the codebase accidentially break something by fixing something else, I'm not too keen on just plunging ahead with proposed fixes that might have even more severe unintended consequences. If js-beautify doesn't get fixed by someone else, I'll have to make a decision whether to get more involved with its codebase or whether to switch this project here to some other tool.\r\n\r\n> We'd be happy to discuss taking code samples from this codebase as baselines to check for changes, too.\r\n\r\nThe examples I posted in my issue reports were adapted from code I found in our codebase.\r\n\r\nFeel free to add a test which checks out our codebase and runs the latest version of js-beautify on it. If that results in any difference, decide whether the change is accidential or intentional. In the former case, you can fix it before your next release. In the latter case, we'd be happy to receive a pull request asking to adjust our code and bump the js-beautify version number. But I guess you shouldn't make that a part of some automated test suite to be performed on each commit. Merely a suggested step prior to a release. The steps to do such a test would be\r\n\r\n```sh\r\ngit clone https://github.com/CindyJS/CindyJS.git\r\ncd CindyJS\r\nnpm install\r\nnpm install js-beautify@‹latest commit›\r\nnode make beautified\r\n```\r\n\r\nOf course you can come up with a way to re-use an existing clone instead of getting a new one for every build. In that case you'll have to do a `git checkout -- .` to clean up possible changes introduced by js-beautify. You might also want to symlink the `js-beautify` directory instead of doing an `npm install` for it, so that you can test a version before pushing it to GitHub. I think you get the idea."}}
{"actor":{"display_login":"gagern"}}