{"action":"created","issue":{"url":"https://api.github.com/repos/theforeman/foreman/issues/2814","labels_url":"https://api.github.com/repos/theforeman/foreman/issues/2814/labels{/name}","comments_url":"https://api.github.com/repos/theforeman/foreman/issues/2814/comments","events_url":"https://api.github.com/repos/theforeman/foreman/issues/2814/events","html_url":"https://github.com/theforeman/foreman/pull/2814","id":111126065,"number":2814,"title":"fixes #12147 - rails 3 compatible updates for rails4","user":{"login":"unorthodoxgeek","id":706359,"avatar_url":"https://avatars.githubusercontent.com/u/706359?v=3","gravatar_id":"","url":"https://api.github.com/users/unorthodoxgeek","html_url":"https://github.com/unorthodoxgeek","followers_url":"https://api.github.com/users/unorthodoxgeek/followers","following_url":"https://api.github.com/users/unorthodoxgeek/following{/other_user}","gists_url":"https://api.github.com/users/unorthodoxgeek/gists{/gist_id}","starred_url":"https://api.github.com/users/unorthodoxgeek/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/unorthodoxgeek/subscriptions","organizations_url":"https://api.github.com/users/unorthodoxgeek/orgs","repos_url":"https://api.github.com/users/unorthodoxgeek/repos","events_url":"https://api.github.com/users/unorthodoxgeek/events{/privacy}","received_events_url":"https://api.github.com/users/unorthodoxgeek/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/theforeman/foreman/labels/Needs%20testing","name":"Needs testing","color":"fbca04"},{"url":"https://api.github.com/repos/theforeman/foreman/labels/Not%20yet%20reviewed","name":"Not yet reviewed","color":"009800"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-10-13T07:58:29Z","updated_at":"2015-10-14T08:34:46Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/theforeman/foreman/pulls/2814","html_url":"https://github.com/theforeman/foreman/pull/2814","diff_url":"https://github.com/theforeman/foreman/pull/2814.diff","patch_url":"https://github.com/theforeman/foreman/pull/2814.patch"},"body":""},"comment":{"url":"https://api.github.com/repos/theforeman/foreman/issues/comments/147974386","html_url":"https://github.com/theforeman/foreman/pull/2814#issuecomment-147974386","issue_url":"https://api.github.com/repos/theforeman/foreman/issues/2814","id":147974386,"user":{"login":"eLobato","id":598891,"avatar_url":"https://avatars.githubusercontent.com/u/598891?v=3","gravatar_id":"","url":"https://api.github.com/users/eLobato","html_url":"https://github.com/eLobato","followers_url":"https://api.github.com/users/eLobato/followers","following_url":"https://api.github.com/users/eLobato/following{/other_user}","gists_url":"https://api.github.com/users/eLobato/gists{/gist_id}","starred_url":"https://api.github.com/users/eLobato/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eLobato/subscriptions","organizations_url":"https://api.github.com/users/eLobato/orgs","repos_url":"https://api.github.com/users/eLobato/repos","events_url":"https://api.github.com/users/eLobato/events{/privacy}","received_events_url":"https://api.github.com/users/eLobato/received_events","type":"User","site_admin":false},"created_at":"2015-10-14T08:34:46Z","updated_at":"2015-10-14T08:34:46Z","body":"@unorthodoxgeek In general I know why these changes were needed and they are necessary in Rails 4. We should try to merge this knowing why and what exactly are we fixing. A good way to do so and get things in faster, and make reviewer's life easy (which makes developer life easier as well) would be to:\r\n\r\n1 - Find if other parts of the code need more fixes like this (if any).\r\n2 - Submit a PR with the changes, explaining in the commit description why exactly a fix is necessary, why is it failing on Rails 4, in what way, and how we fix it. I know https://github.com/theforeman/foreman/pull/2358 didn't do it and it was a mistake as we don't know exactly what was that commit for. I tried to amend this mistake and submit things in a more sensible way from there onwards.\r\n\r\nFor instance in this PR, the `.reload` fix has :zero: to do with the :factory: Factory fix, and they have nothing to do with the ensure_no_cycle fix. I'm fine merging some of these individually right away, like factories or seeds_test (when there's a good description!), but some others like `ensure_no_cycle` require a bit more testing and reviewing - I wrote that code and I feel like it needs closer inspection :smile: \r\n\r\nAll in all what I'm trying to say is that if this PR is split into a few others with good descriptions each PR will be a lot less risk and so much easier to understand in the end it gets merged faster. Does that sound good? You could keep the same issue number even, and modify the title and description for one of the issues. "}}