RESOLVED FIXED 183184
[webkitpy] Remove concept of 'future' versions
https://bugs.webkit.org/show_bug.cgi?id=183184
Summary [webkitpy] Remove concept of 'future' versions
Jonathan Bedard
Reported 2018-02-27 15:01:29 PST
Remove the idea of 'future' from Mac and iOS versions. Instead, use decrementing version numbers and inserted apple_additions() to handle this.
Attachments
Patch (16.96 KB, patch)
2018-02-27 15:07 PST, Jonathan Bedard
no flags
Patch (20.05 KB, patch)
2018-02-27 16:58 PST, Jonathan Bedard
no flags
Patch for landing (20.10 KB, patch)
2018-02-27 17:28 PST, Jonathan Bedard
no flags
Follow-up fix (3.67 KB, patch)
2018-02-28 11:26 PST, Jonathan Bedard
no flags
Follow-up fix 2 (4.64 KB, patch)
2018-02-28 17:09 PST, Jonathan Bedard
no flags
Patch (4.66 KB, patch)
2018-02-28 17:29 PST, Jonathan Bedard
no flags
Radar WebKit Bug Importer
Comment 1 2018-02-27 15:03:45 PST
Jonathan Bedard
Comment 2 2018-02-27 15:07:02 PST
Jonathan Bedard
Comment 3 2018-02-27 16:58:36 PST
Aakash Jain
Comment 4 2018-02-27 17:21:42 PST
Comment on attachment 334715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334715&action=review > Tools/Scripts/webkitpy/port/mac.py:78 > + while temp_version != self.CURRENT_VERSION: This will become infinite loop if someone incorrectly set self._os_version.major to anything other than 10. Please add an assert to make sure that for macos major version is always 10.
Aakash Jain
Comment 5 2018-02-27 17:27:29 PST
Comment on attachment 334715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334715&action=review Looks good. > Tools/ChangeLog:3 > + Remove concept of 'future' Can make the bug title more descriptive, like "Remove concept of 'future' from TestExpectations".
Jonathan Bedard
Comment 6 2018-02-27 17:28:44 PST
Created attachment 334718 [details] Patch for landing
WebKit Commit Bot
Comment 7 2018-02-27 18:06:03 PST
Comment on attachment 334718 [details] Patch for landing Clearing flags on attachment: 334718 Committed r229085: <https://trac.webkit.org/changeset/229085>
WebKit Commit Bot
Comment 8 2018-02-27 18:06:05 PST
All reviewed patches have been landed. Closing bug.
Simon Fraser (smfr)
Comment 9 2018-02-27 18:20:32 PST
This bug could have had a more descriptive title.
Daniel Bates
Comment 10 2018-02-27 21:07:49 PST
(In reply to Simon Fraser (smfr) from comment #9) > This bug could have had a more descriptive title. You’re not the only one to point this out. It was pointed out in comment 5.
Aakash Jain
Comment 11 2018-02-28 07:43:17 PST
I think it was my oversight. I was doing over the shoulder code review with Jonathan and he incorporated all my verbal review comments. I commented about bug title here but forgot to tell Jonathan. He was using webkit-patch command to upload patch so probably didn't notice my bugzilla comment.
Jonathan Bedard
Comment 12 2018-02-28 11:26:05 PST
Reopening to attach new patch.
Jonathan Bedard
Comment 13 2018-02-28 11:26:06 PST
Created attachment 334753 [details] Follow-up fix
Jonathan Bedard
Comment 14 2018-02-28 11:27:16 PST
(In reply to Jonathan Bedard from comment #13) > Created attachment 334753 [details] > Follow-up fix Missed a non-obvious dependency on future in the factory.
David Kilzer (:ddkilzer)
Comment 15 2018-02-28 11:30:40 PST
Comment on attachment 334753 [details] Follow-up fix View in context: https://bugs.webkit.org/attachment.cgi?id=334753&action=review r=me > Tools/ChangeLog:9 > + The factory also relies off of future in a non-obvious way. Nit: "off of" => "on"
Jonathan Bedard
Comment 16 2018-02-28 11:35:41 PST
Follow-up fix committed <https://trac.webkit.org/changeset/229099>.
Jonathan Bedard
Comment 17 2018-02-28 17:09:55 PST
Created attachment 334787 [details] Follow-up fix 2
Jonathan Bedard
Comment 18 2018-02-28 17:29:40 PST
WebKit Commit Bot
Comment 19 2018-02-28 18:09:44 PST
Comment on attachment 334789 [details] Patch Clearing flags on attachment: 334789 Committed r229116: <https://trac.webkit.org/changeset/229116>
WebKit Commit Bot
Comment 20 2018-02-28 18:09:45 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.