Issue 6608 - Removed CI refs from Target settings for ABB and Share Extension

Issue 6608 - Removed CI refs from Target settings for ABB and Share Extension

[dean, 2018-04-06 00:03:56]

https://codereview.adblockplus.org/29743618/diff/29743619/AdBlockBrowser/Info...
File AdBlockBrowser/Info.plist (right):

https://codereview.adblockplus.org/29743618/diff/29743619/AdBlockBrowser/Info...
AdBlockBrowser/Info.plist:54: <string>05_04_2018</string>
Same as the other file, I'd suggest having this value mirror
CFBundleShortVersionString value rather than a date. Once CI is setup, we can
look into a more meaningful build number system.

https://codereview.adblockplus.org/29743618/diff/29743619/AdBlockBrowser/Info...
AdBlockBrowser/Info.plist:90: </dict>
Silly question - Was the changes to the formatting of this file done
automatically?

https://codereview.adblockplus.org/29743618/diff/29743619/ExtensionShare/Info...
File ExtensionShare/Info.plist (right):

https://codereview.adblockplus.org/29743618/diff/29743619/ExtensionShare/Info...
ExtensionShare/Info.plist:28: <string>05_04_2018</string>
As we're not currently utilising CFBundleVersion, I'd suggest having it mirror
the CFBundleShortVersionString rather than hardcoding it to a date.

[a.shephard, 2018-04-06 09:58:20]

Updated to Patchset 2.
I've changed it so that both values (CFBundleVersion & CFBundleShortVersion) are
matching - no datestamp.

The plists I've also formatted (indented) according to our agreed Xcode
settings.

[d108, 2018-04-06 23:23:59]

https://codereview.adblockplus.org/29743618/diff/29744585/AdBlockBrowser/Info...
File AdBlockBrowser/Info.plist (right):

https://codereview.adblockplus.org/29743618/diff/29744585/AdBlockBrowser/Info...
AdBlockBrowser/Info.plist:54: <string>2.0.0</string>
I suggest reverting CFBundleVersion to be the build number for now as this will
be needed for the app store. I also suggest this be a user defined value such as
$CORE_BUILD_NUMBER so that when we use custom configurations that we, or CI,can
assign different values to each build type such as devbuild, release, etc.

https://codereview.adblockplus.org/29743618/diff/29744585/ExtensionShare/Info...
File ExtensionShare/Info.plist (right):

https://codereview.adblockplus.org/29743618/diff/29744585/ExtensionShare/Info...
ExtensionShare/Info.plist:28: <string>2.0.0</string>
I suggest reverting CFBundleVersion to be the build number for now as this will
be needed for the app store. I also suggest this be a user defined value such as
$CORE_BUILD_NUMBER so that when we use custom configurations that we, or CI,can
assign different values to each build type such as devbuild, release, etc.

[a.shephard, 2018-04-18 11:24:03]

On 2018/04/06 23:23:59, d108 wrote:
>
https://codereview.adblockplus.org/29743618/diff/29744585/AdBlockBrowser/Info...
> File AdBlockBrowser/Info.plist (right):
> 
>
https://codereview.adblockplus.org/29743618/diff/29744585/AdBlockBrowser/Info...
> AdBlockBrowser/Info.plist:54: <string>2.0.0</string>
> I suggest reverting CFBundleVersion to be the build number for now as this
will
> be needed for the app store. I also suggest this be a user defined value such
as
> $CORE_BUILD_NUMBER so that when we use custom configurations that we, or
CI,can
> assign different values to each build type such as devbuild, release, etc.
> 
>
https://codereview.adblockplus.org/29743618/diff/29744585/ExtensionShare/Info...
> File ExtensionShare/Info.plist (right):
> 
>
https://codereview.adblockplus.org/29743618/diff/29744585/ExtensionShare/Info...
> ExtensionShare/Info.plist:28: <string>2.0.0</string>
> I suggest reverting CFBundleVersion to be the build number for now as this
will
> be needed for the app store. I also suggest this be a user defined value such
as
> $CORE_BUILD_NUMBER so that when we use custom configurations that we, or
CI,can
> assign different values to each build type such as devbuild, release, etc.

Could we delay this change until we move over to GitLab? I removed the
CORE_BUILD_NUMBER so that we could start fresh with the CI implementation once
we land on GitLab.

[d108, 2018-04-19 02:36:37]

On 2018/04/18 11:24:03, a.shephard wrote:
> On 2018/04/06 23:23:59, d108 wrote:
> >
>
https://codereview.adblockplus.org/29743618/diff/29744585/AdBlockBrowser/Info...
> > File AdBlockBrowser/Info.plist (right):
> > 
> >
>
https://codereview.adblockplus.org/29743618/diff/29744585/AdBlockBrowser/Info...
> > AdBlockBrowser/Info.plist:54: <string>2.0.0</string>
> > I suggest reverting CFBundleVersion to be the build number for now as this
> will
> > be needed for the app store. I also suggest this be a user defined value
such
> as
> > $CORE_BUILD_NUMBER so that when we use custom configurations that we, or
> CI,can
> > assign different values to each build type such as devbuild, release, etc.
> > 
> >
>
https://codereview.adblockplus.org/29743618/diff/29744585/ExtensionShare/Info...
> > File ExtensionShare/Info.plist (right):
> > 
> >
>
https://codereview.adblockplus.org/29743618/diff/29744585/ExtensionShare/Info...
> > ExtensionShare/Info.plist:28: <string>2.0.0</string>
> > I suggest reverting CFBundleVersion to be the build number for now as this
> will
> > be needed for the app store. I also suggest this be a user defined value
such
> as
> > $CORE_BUILD_NUMBER so that when we use custom configurations that we, or
> CI,can
> > assign different values to each build type such as devbuild, release, etc.
> 
> Could we delay this change until we move over to GitLab? I removed the
> CORE_BUILD_NUMBER so that we could start fresh with the CI implementation once
> we land on GitLab.

Perhaps the option here, in planning for the move, is to set CFBundleVersion to
$(BUILD_NUMBER) as is the case with ABP. The future CI config will then be able
to use the same symbols for ABB and ABP.

[a.shephard, 2018-04-19 09:11:08]

Put it to BUILD_NUMBER for now until we move to GitLab and work with CI.

[dean, 2018-04-20 00:25:56]

On 2018/04/19 09:11:08, a.shephard wrote:
> Put it to BUILD_NUMBER for now until we move to GitLab and work with CI.

LGTM

[d108, 2018-04-21 04:07:28]

On 2018/04/19 09:11:08, a.shephard wrote:
> Put it to BUILD_NUMBER for now until we move to GitLab and work with CI.

Since there seems to be some future intention here, this issue would probably be
better as a tracked issue.

[a.shephard, 2018-04-23 08:38:07]

Added BUILD_NUMBER property to the Build Settings and created an issue number.
Will probably just leave the issue number to this (Remove CI) and make a new
issue for adding it again.

[d108, 2018-04-23 22:00:17]

On 2018/04/23 08:38:07, a.shephard wrote:
> Added BUILD_NUMBER property to the Build Settings and created an issue number.
> Will probably just leave the issue number to this (Remove CI) and make a new
> issue for adding it again.

LGTM

[dean, 2018-04-23 22:41:49]

On 2018/04/23 22:00:17, d108 wrote:
> On 2018/04/23 08:38:07, a.shephard wrote:
> > Added BUILD_NUMBER property to the Build Settings and created an issue
number.
> > Will probably just leave the issue number to this (Remove CI) and make a new
> > issue for adding it again.
> 
> LGTM

With exception to the change to the SwiftLint script, LGTM.

[dean, 2018-04-23 22:42:13]

with exception to the change in the SwiftLint script - LGTM.

https://codereview.adblockplus.org/29743618/diff/29759576/AdblockBrowser.xcod...
File AdblockBrowser.xcodeproj/project.pbxproj (right):

https://codereview.adblockplus.org/29743618/diff/29759576/AdblockBrowser.xcod...
AdblockBrowser.xcodeproj/project.pbxproj:1239: shellScript =
"exit\nSWIFTLINT=\"/usr/local/bin/swiftlint\"\nif [ -f $SWIFTLINT ]\nthen\n   
$SWIFTLINT\nelse\n    echo \"warning: SwiftLint not installed, download from
https://github.com/realm/SwiftLint\"\nfi";
I think this change needs to be rolled back and may be committed in error.