Issue 10950010: Re-save XIB files using Xcode 4.4.1 on Mac OS X 10.8.1.

Issue 10950010: Re-save XIB files using Xcode 4.4.1 on Mac OS X 10.8.1. (Closed)

Created:
8 years, 3 months ago by Robert Sesek

Modified:
8 years, 3 months ago

Reviewers:
Mark Mentovai

CC:
chromium-reviews

Base URL:
http://git.chromium.org/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

Re-save XIB files using Xcode 4.4.1 on Mac OS X 10.8.1. This also adds a PRESUBMIT check to enforce the policy described here: http://dev.chromium.org/developers/design-documents/mac-xib-files BUG=149763 TEST=Build chrome target on 10.6 with Xcode 3.2. Chromium.app launches and works. TBR=mark@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=157690

Patch Set 2 : GTMUILocalizer #

Total comments: 14

Patch Set 3 : Address comments #

Total comments: 1

Patch Set 4 : 80cols #

Created: 8 years, 3 months ago

(Patch set is too large to download)

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+7137 lines, -42834 lines)			Patch
M	chrome/app/nibs/AboutIPC.xib	View	1 2 3	58 chunks	+249 lines, -966 lines	0 comments	Download
M	chrome/app/nibs/AvatarMenuItem.xib	View	1 2 3	29 chunks	+77 lines, -927 lines	0 comments	Download
M	chrome/app/nibs/BookmarkAllTabs.xib	View	1 2 3	43 chunks	+185 lines, -987 lines	0 comments	Download
M	chrome/app/nibs/BookmarkBar.xib	View	1 2 3	49 chunks	+279 lines, -1036 lines	0 comments	Download
M	chrome/app/nibs/BookmarkBarFolderWindow.xib	View	1 2 3	41 chunks	+187 lines, -910 lines	0 comments	Download
M	chrome/app/nibs/BookmarkBubble.xib	View	1 2 3	43 chunks	+156 lines, -916 lines	0 comments	Download
M	chrome/app/nibs/BookmarkEditor.xib	View	1 2 3	45 chunks	+242 lines, -1003 lines	0 comments	Download
M	chrome/app/nibs/BookmarkNameFolder.xib	View	1 2 3	28 chunks	+79 lines, -799 lines	0 comments	Download
M	chrome/app/nibs/BrowserWindow.xib	View	1 2 3	23 chunks	+130 lines, -837 lines	0 comments	Download
M	chrome/app/nibs/ChromeToMobileBubble.xib	View	1 2 3	38 chunks	+114 lines, -766 lines	0 comments	Download
M	chrome/app/nibs/CollectedCookies.xib	View	1 2 3	47 chunks	+436 lines, -1031 lines	0 comments	Download
M	chrome/app/nibs/ContentBlockedCookies.xib	View	1 2 3	26 chunks	+293 lines, -822 lines	0 comments	Download
M	chrome/app/nibs/ContentBlockedGeolocation.xib	View	1 2 3	33 chunks	+116 lines, -817 lines	0 comments	Download
M	chrome/app/nibs/ContentBlockedImages.xib	View	1 2 3	35 chunks	+129 lines, -832 lines	0 comments	Download
M	chrome/app/nibs/ContentBlockedJavaScript.xib	View	1 2 3	35 chunks	+129 lines, -832 lines	0 comments	Download
M	chrome/app/nibs/ContentBlockedMixedScript.xib	View	1 2 3	35 chunks	+105 lines, -911 lines	0 comments	Download
M	chrome/app/nibs/ContentBlockedPlugins.xib	View	1 2 3	39 chunks	+118 lines, -899 lines	0 comments	Download
M	chrome/app/nibs/ContentBlockedPopups.xib	View	1 2 3	35 chunks	+129 lines, -832 lines	0 comments	Download
M	chrome/app/nibs/ContentProtocolHandlers.xib	View	1 2 3	37 chunks	+131 lines, -840 lines	0 comments	Download
M	chrome/app/nibs/CookieDetailsView.xib	View	1 2 3	141 chunks	+707 lines, -1302 lines	0 comments	Download
M	chrome/app/nibs/DownloadItem.xib	View	1 2 3	35 chunks	+92 lines, -827 lines	0 comments	Download
M	chrome/app/nibs/DownloadShelf.xib	View	1 2 3	37 chunks	+71 lines, -619 lines	0 comments	Download
M	chrome/app/nibs/EditSearchEngine.xib	View	1 2 3	38 chunks	+122 lines, -882 lines	0 comments	Download
M	chrome/app/nibs/ExtensionInstallPrompt.xib	View	1 2 3	37 chunks	+111 lines, -961 lines	0 comments	Download
M	chrome/app/nibs/ExtensionInstallPromptBundle.xib	View	1 2 3	35 chunks	+106 lines, -1020 lines	0 comments	Download
M	chrome/app/nibs/ExtensionInstallPromptInline.xib	View	1 2 3	42 chunks	+135 lines, -1035 lines	0 comments	Download
M	chrome/app/nibs/ExtensionInstallPromptNoWarnings.xib	View	1 2 3	29 chunks	+99 lines, -831 lines	0 comments	Download
M	chrome/app/nibs/ExtensionInstalledBubble.xib	View	1 2 3	40 chunks	+107 lines, -724 lines	0 comments	Download
M	chrome/app/nibs/ExtensionInstalledBubbleBundle.xib	View	1 2 3	22 chunks	+83 lines, -702 lines	0 comments	Download
M	chrome/app/nibs/FindBar.xib	View	1 2 3	26 chunks	+57 lines, -607 lines	0 comments	Download
M	chrome/app/nibs/FirstRunBubble.xib	View	1 2 3	21 chunks	+70 lines, -854 lines	0 comments	Download
M	chrome/app/nibs/FirstRunDialog.xib	View	1 2 3	42 chunks	+136 lines, -745 lines	0 comments	Download
M	chrome/app/nibs/FullscreenExitBubble.xib	View	1 2 3	31 chunks	+95 lines, -884 lines	0 comments	Download
M	chrome/app/nibs/GlobalErrorBubble.xib	View	1 2 3	28 chunks	+144 lines, -842 lines	0 comments	Download
M	chrome/app/nibs/HttpAuthLoginSheet.xib	View	1 2 3	35 chunks	+108 lines, -840 lines	0 comments	Download
M	chrome/app/nibs/HungRendererDialog.xib	View	1 2 3	36 chunks	+119 lines, -868 lines	0 comments	Download
M	chrome/app/nibs/ImportProgressDialog.xib	View	1 2 3	26 chunks	+163 lines, -892 lines	0 comments	Download
M	chrome/app/nibs/InfoBar.xib	View	1 2 3	29 chunks	+62 lines, -647 lines	0 comments	Download
M	chrome/app/nibs/InfoBarContainer.xib	View	1 2 3	6 chunks	+19 lines, -672 lines	0 comments	Download
M	chrome/app/nibs/MainMenu.xib	View	1 2 3	38 chunks	+114 lines, -1085 lines	0 comments	Download
M	chrome/app/nibs/Notification.xib	View	1 2 3	18 chunks	+60 lines, -783 lines	0 comments	Download
M	chrome/app/nibs/OneClickSigninBubble.xib	View	1 2 3	34 chunks	+127 lines, -807 lines	0 comments	Download
A	chrome/app/nibs/PRESUBMIT.py	View	1 2 3	1 chunk	+84 lines, -0 lines	0 comments	Download
M	chrome/app/nibs/Panel.xib	View	1 2 3	25 chunks	+88 lines, -910 lines	0 comments	Download
M	chrome/app/nibs/PreviewableContents.xib	View	1 2 3	7 chunks	+21 lines, -673 lines	0 comments	Download
M	chrome/app/nibs/SadTab.xib	View	1 2 3	22 chunks	+70 lines, -788 lines	0 comments	Download
M	chrome/app/nibs/SaveAccessoryView.xib	View	1 2 3	16 chunks	+42 lines, -787 lines	0 comments	Download
M	chrome/app/nibs/SpeechRecognitionBubble.xib	View	1 2 3	24 chunks	+211 lines, -828 lines	0 comments	Download
M	chrome/app/nibs/TabView.xib	View	1 2 3	25 chunks	+63 lines, -637 lines	0 comments	Download
M	chrome/app/nibs/TaskManager.xib	View	1 2 3	34 chunks	+118 lines, -859 lines	0 comments	Download
M	chrome/app/nibs/Toolbar.xib	View	1 2 3	40 chunks	+108 lines, -841 lines	0 comments	Download
M	chrome/app/nibs/WrenchMenu.xib	View	1 2 3	36 chunks	+133 lines, -119 lines	0 comments	Download
M	chrome/chrome_nibs.gyp	View	1 2 3	4 chunks	+8 lines, -0 lines	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

Mark Mentovai

http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT.py File chrome/app/nibs/PRESUBMIT.py (right): http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT.py#newcode1 chrome/app/nibs/PRESUBMIT.py:1: # Copyright (c) 2011 The Chromium Authors. All rights ...

8 years, 3 months ago (2012-09-18 20:16:59 UTC) #2

Robert Sesek

8 years, 3 months ago (2012-09-18 20:42:10 UTC) #3

http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT.py
File chrome/app/nibs/PRESUBMIT.py (right):

http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT....
chrome/app/nibs/PRESUBMIT.py:1: # Copyright (c) 2011 The Chromium Authors. All
rights reserved.
On 2012/09/18 20:16:59, Mark Mentovai wrote:
> What year, now?

Oops.

http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT....
chrome/app/nibs/PRESUBMIT.py:14: HUMAN_DARWIN_VERSION = '10.8.x'
On 2012/09/18 20:16:59, Mark Mentovai wrote:
> Technically incorrect if you’re requiring 10.8.1 but not allowing 10.8. The
> human message can say “10.8.x, x >= 1”

Done.

http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT....
chrome/app/nibs/PRESUBMIT.py:19: MAXIMUM_IB_VERSION = 2800  # Xcode 4.5.x.
On 2012/09/18 20:16:59, Mark Mentovai wrote:
> For the name of the variable to make sense, I think it should be 2800 - 1, and
> the >= check in the code below should be >.

The earliest 4.5 prerelease seed I found had build 2814, so 2800 is ok.

http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT....
chrome/app/nibs/PRESUBMIT.py:20: HUMAN_IB_VERSION = '4.4.x'
On 2012/09/18 20:16:59, Mark Mentovai wrote:
> Similar to before, technically incorrect if you’re requiring 4.4.1 but not
> allowing 4.4.

Done.

http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT....
chrome/app/nibs/PRESUBMIT.py:38: for line in xib.NewContents():
On 2012/09/18 20:16:59, Mark Mentovai wrote:
> Why don’t you have your REs anchored with ^ and $, and instead of scanning
over
> the file line by line, just let that RE crank on NewContents, with the right
> RE_MULTILINE setting to make it work properly?

The AffectedFile only provides iterators to the lines, not the content blob.

http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT....
chrome/app/nibs/PRESUBMIT.py:53: if int(system_version[0:2]) !=
ALLOWED_DARWIN_VERSION:
On 2012/09/18 20:16:59, Mark Mentovai wrote:
> Rather than system_version[0:2] and system_version[2], why don’t you come up
> with a better RE for SYSTEM_VERSION_RE that gives you more (two?) match groups
> to pull the stuff out of. Then you don’t need the silly len(system_version) <
3
> check two lines above either.

Done.

http://codereview.chromium.org/10950010/diff/14005/chrome/app/nibs/PRESUBMIT....
chrome/app/nibs/PRESUBMIT.py:68: 'XIB files need to be saved on Mac OS X ' +
HUMAN_DARWIN_VERSION,
On 2012/09/18 20:16:59, Mark Mentovai wrote:
> Can the message say which xib tripped the check? Same for the IB version
check.

That's what items= does.

Mark Mentovai

LGTM http://codereview.chromium.org/10950010/diff/8013/chrome/app/nibs/PRESUBMIT.py File chrome/app/nibs/PRESUBMIT.py (right): http://codereview.chromium.org/10950010/diff/8013/chrome/app/nibs/PRESUBMIT.py#newcode23 chrome/app/nibs/PRESUBMIT.py:23: r'<string key="IBDocument\.SystemVersion">([0-9]{,2})([A-Z])([0-9]+)</string>' I woulda used pluses, which has ...

8 years, 3 months ago (2012-09-18 20:44:35 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsesek@chromium.org/10950010/16001

8 years, 3 months ago (2012-09-19 23:26:24 UTC) #5

commit-bot: I haz the power

Presubmit check for 10950010-16001 failed and returned exit status 1. Running presubmit commit checks ...

8 years, 3 months ago (2012-09-19 23:27:17 UTC) #6

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsesek@chromium.org/10950010/16001

8 years, 3 months ago (2012-09-19 23:27:54 UTC) #7

Change committed as 157690

Expand Messages | Collapse Messages