{"action":"created","issue":{"url":"https://api.github.com/repos/fvwmorg/fvwm/issues/9","repository_url":"https://api.github.com/repos/fvwmorg/fvwm","labels_url":"https://api.github.com/repos/fvwmorg/fvwm/issues/9/labels{/name}","comments_url":"https://api.github.com/repos/fvwmorg/fvwm/issues/9/comments","events_url":"https://api.github.com/repos/fvwmorg/fvwm/issues/9/events","html_url":"https://github.com/fvwmorg/fvwm/pull/9","id":168781911,"number":9,"title":"Unrolled P(n) macros. Now prototypes are explicitly defined.","user":{"login":"krazeivan","id":489819,"avatar_url":"https://avatars.githubusercontent.com/u/489819?v=3","gravatar_id":"","url":"https://api.github.com/users/krazeivan","html_url":"https://github.com/krazeivan","followers_url":"https://api.github.com/users/krazeivan/followers","following_url":"https://api.github.com/users/krazeivan/following{/other_user}","gists_url":"https://api.github.com/users/krazeivan/gists{/gist_id}","starred_url":"https://api.github.com/users/krazeivan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/krazeivan/subscriptions","organizations_url":"https://api.github.com/users/krazeivan/orgs","repos_url":"https://api.github.com/users/krazeivan/repos","events_url":"https://api.github.com/users/krazeivan/events{/privacy}","received_events_url":"https://api.github.com/users/krazeivan/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-08-02T01:52:00Z","updated_at":"2016-08-02T07:03:44Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/fvwmorg/fvwm/pulls/9","html_url":"https://github.com/fvwmorg/fvwm/pull/9","diff_url":"https://github.com/fvwmorg/fvwm/pull/9.diff","patch_url":"https://github.com/fvwmorg/fvwm/pull/9.patch"},"body":"I hope I was correct in merging my topic branch with my master before opening a pull request... Please let me know if you want it done differently, I'm wasn't sure with the directions posted on the fvwm.org site."},"comment":{"url":"https://api.github.com/repos/fvwmorg/fvwm/issues/comments/236818953","html_url":"https://github.com/fvwmorg/fvwm/pull/9#issuecomment-236818953","issue_url":"https://api.github.com/repos/fvwmorg/fvwm/issues/9","id":236818953,"user":{"login":"ThomasAdam","id":101281,"avatar_url":"https://avatars.githubusercontent.com/u/101281?v=3","gravatar_id":"","url":"https://api.github.com/users/ThomasAdam","html_url":"https://github.com/ThomasAdam","followers_url":"https://api.github.com/users/ThomasAdam/followers","following_url":"https://api.github.com/users/ThomasAdam/following{/other_user}","gists_url":"https://api.github.com/users/ThomasAdam/gists{/gist_id}","starred_url":"https://api.github.com/users/ThomasAdam/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ThomasAdam/subscriptions","organizations_url":"https://api.github.com/users/ThomasAdam/orgs","repos_url":"https://api.github.com/users/ThomasAdam/repos","events_url":"https://api.github.com/users/ThomasAdam/events{/privacy}","received_events_url":"https://api.github.com/users/ThomasAdam/received_events","type":"User","site_admin":false},"created_at":"2016-08-02T07:03:44Z","updated_at":"2016-08-02T07:03:44Z","body":"Thanks for this.  In answer to your question, the workflow for submitting pull-requests and where you should do that work is documented here:\r\n\r\nhttps://github.com/fvwmorg/fvwm/blob/master/docs/DEVELOPERS.md#submitting-pull-requests\r\n\r\nI see you've made these changes directly on master---you shouldn't really do that.  I would instead:\r\n\r\n```\r\ngit checkout master && git checkout -b unroll-p-macro\r\ngit checkout - && git reset --hard @{u}\r\ngit checkout -\r\n\r\n# Now you're on the `unroll-p-macro` branch.\r\n```\r\n\r\nWhich will have created a new branch from your copy of master (containing your P(n) macro changes), called `unroll-p-macro`, which you can then use for further changes.\r\n\r\nAgain, have a look at the link mentioned earlier about how to submit pull-requests.\r\n\r\nNow for some comments:\r\n\r\nI think the commit message could do with some tidy-up.  You might think that sounds a little draconian, but it's important when you're making a change to say *why* you're making the change.   I can see you've very helpfully started to take things off my TODO list which I'm really grateful for.  So let's try and reword it:\r\n\r\n```\r\nUnroll P(n) macro for explicit prototyping\r\n\r\nHiding the function prototype behind a macro masks any changes which might not be common in the future, and doesn't make for clarity as to what the functions are doing.  Unroll the prototype and put the function definitions back in full.\r\n```\r\n\r\nWhat do you think?  You can then make those changes and push them out, and this PR should survive, with the added bonus that your work is now in its own topic branch."}}
{"actor":{"display_login":"ThomasAdam"}}