{"action":"created","issue":{"url":"https://api.github.com/repos/plotly/plotly.js/issues/3168","repository_url":"https://api.github.com/repos/plotly/plotly.js","labels_url":"https://api.github.com/repos/plotly/plotly.js/issues/3168/labels{/name}","comments_url":"https://api.github.com/repos/plotly/plotly.js/issues/3168/comments","events_url":"https://api.github.com/repos/plotly/plotly.js/issues/3168/events","html_url":"https://github.com/plotly/plotly.js/pull/3168","id":374462110,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI2MTU5NTgy","number":3168,"title":"Fix scatter fill regression","user":{"login":"etpinard","id":6675409,"node_id":"MDQ6VXNlcjY2NzU0MDk=","avatar_url":"https://avatars0.githubusercontent.com/u/6675409?v=4","gravatar_id":"","url":"https://api.github.com/users/etpinard","html_url":"https://github.com/etpinard","followers_url":"https://api.github.com/users/etpinard/followers","following_url":"https://api.github.com/users/etpinard/following{/other_user}","gists_url":"https://api.github.com/users/etpinard/gists{/gist_id}","starred_url":"https://api.github.com/users/etpinard/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/etpinard/subscriptions","organizations_url":"https://api.github.com/users/etpinard/orgs","repos_url":"https://api.github.com/users/etpinard/repos","events_url":"https://api.github.com/users/etpinard/events{/privacy}","received_events_url":"https://api.github.com/users/etpinard/received_events","type":"User","site_admin":false},"labels":[{"id":330750748,"node_id":"MDU6TGFiZWwzMzA3NTA3NDg=","url":"https://api.github.com/repos/plotly/plotly.js/labels/status:%20reviewable","name":"status: reviewable","color":"33a02c","default":false},{"id":282886923,"node_id":"MDU6TGFiZWwyODI4ODY5MjM=","url":"https://api.github.com/repos/plotly/plotly.js/labels/type:%20bug","name":"type: bug","color":"e31a1c","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-10-26T16:41:11Z","updated_at":"2018-10-26T17:32:10Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/plotly/plotly.js/pulls/3168","html_url":"https://github.com/plotly/plotly.js/pull/3168","diff_url":"https://github.com/plotly/plotly.js/pull/3168.diff","patch_url":"https://github.com/plotly/plotly.js/pull/3168.patch"},"body":"This PR fixes an unreleased regression originating from https://github.com/plotly/plotly.js/pull/3087. I'd like to get this in before the 1.42.0 release.\r\n\r\nScatter fills on master currently do not restyle correctly and lead to these logs:\r\n\r\n![image](https://user-images.githubusercontent.com/6675409/47579989-99a6f400-d91b-11e8-827d-a2c9b2d65e7c.png)\r\n\r\nbut **no** failed tests. That's because those logs come from:\r\n\r\nhttps://github.com/plotly/plotly.js/blob/0a310c99455bf7389f4bb891392c806b59d93d47/src/components/drawing/index.js#L201-L207\r\n\r\nwhich do **not** make `failTest` throw, leading to false-positive tests.\r\n\r\n----------\r\n\r\nThe first commit fixes and :lock:s the bug, and the second commit :hocho:s that try-catch (which is at least 4 years old) which is probably a bit more dangerous, but I'm willing to take that chance. I hope @plotly/plotly_js agrees."},"comment":{"url":"https://api.github.com/repos/plotly/plotly.js/issues/comments/433485034","html_url":"https://github.com/plotly/plotly.js/pull/3168#issuecomment-433485034","issue_url":"https://api.github.com/repos/plotly/plotly.js/issues/3168","id":433485034,"node_id":"MDEyOklzc3VlQ29tbWVudDQzMzQ4NTAzNA==","user":{"login":"alexcjohnson","id":2678795,"node_id":"MDQ6VXNlcjI2Nzg3OTU=","avatar_url":"https://avatars3.githubusercontent.com/u/2678795?v=4","gravatar_id":"","url":"https://api.github.com/users/alexcjohnson","html_url":"https://github.com/alexcjohnson","followers_url":"https://api.github.com/users/alexcjohnson/followers","following_url":"https://api.github.com/users/alexcjohnson/following{/other_user}","gists_url":"https://api.github.com/users/alexcjohnson/gists{/gist_id}","starred_url":"https://api.github.com/users/alexcjohnson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexcjohnson/subscriptions","organizations_url":"https://api.github.com/users/alexcjohnson/orgs","repos_url":"https://api.github.com/users/alexcjohnson/repos","events_url":"https://api.github.com/users/alexcjohnson/events{/privacy}","received_events_url":"https://api.github.com/users/alexcjohnson/received_events","type":"User","site_admin":false},"created_at":"2018-10-26T17:32:10Z","updated_at":"2018-10-26T17:32:10Z","author_association":"CONTRIBUTOR","body":"Good catch! I noticed those logs but studiously ignored them 🙈 - glad someone is paying more attention!!\r\n\r\n> The first commit fixes and 🔒s the bug\r\n\r\nHow does this 🔒 it?\r\n\r\n> the second commit 🔪s that try-catch ... which is probably a bit more dangerous, but I'm willing to take that chance.\r\n\r\nMe too. We have enough tests at this point, don't worry about removing it. As evidenced here, a `try/catch` like this is more likely to paper over a problem than to provide useful behavior!"}}
{"actor":{"display_login":"alexcjohnson"}}