Re: Update on iOS ARC conversion

[Shazron <sh...@gmail.com>]

Using your ARC branch, I've verified that mobile-spec tests pass and
the relevant obj-c unit tests as well. I think this is good to go for
a merge.

On Tue, Jul 31, 2012 at 2:34 PM, Shazron <sh...@gmail.com> wrote:
> Thanks Becky,
> I'll take a look later today or tomorrow morning. Ideally I want the
> plugin upgrade guide in the main docs repo, but until then we should
> put it in the Plugin Upgrade Guide with a note in the Upgrading Guide
> to check it out.
>
> On Tue, Jul 31, 2012 at 2:18 PM, Becky Gibson <gi...@gmail.com> wrote:
>> I've merged in all of the latest iOS commits into my branch.  I updated my
>> commits to remove the commented out code.  Fixed some ARC-conversion memory
>> crashes in Contacts (the bridging to core foundation classes can be
>> tedious).  All of the mobile-spec tests run on the iPhone Simulator and my
>> iPhone 4S.  I also ran my own set of tests.
>>
>> I'd love to check this in before there are more commits to trunk that I
>> have to merge.  It would be great if someone could pull down this code and
>> do a make and create a new project to validate my testing.  I am still
>> running Lion and xcode 4.3.2.
>>
>> What type of document do we need to explain this?  I guess it is just a
>> matter of telling people how to add compile flags to individual plugin
>> files if they want to use non-ARC code with the ARC enabled Cordova code
>> and *.xcodeproj files.   Does this go in the plugin upgrade guide?
>>
>> thanks,
>> -becky
>>
>>
>> On Wed, Jul 25, 2012 at 3:23 PM, Andrew Grieve <ag...@chromium.org> wrote:
>>
>>> I had a look through your diff. It's nice to see all the
>>> retain/release/autoreleases gone, but a bit scary at the same time :P. I
>>> think you're probably fine to delete the commented out parts, and it will
>>> probably make the diff even easier to read (fewer lines in the diff that
>>> way).
>>>
>>> Just from the limited work I've done on FileTransfer, I know that the
>>> mobile-spec tests (automated ones) for it give pretty good coverage.
>>>
>>> Agree that it'd be good to get this in soon so we can maximize testing on
>>> it.
>>>
>>> Good call on leaving JSONKit as-is.
>>>
>>>
>>> Andrew
>>>
>>>
>>> On Tue, Jul 24, 2012 at 4:41 PM, Shazron <sh...@gmail.com> wrote:
>>>
>>> > Thanks Becky!
>>> > I would start with ChildBrowser and then choose any other ones by last
>>> > modified and medium complex
>>> > https://github.com/phonegap/phonegap-plugins/tree/master/iOS (eg
>>> > BarcodeScanner, NavigationBar).
>>> >
>>> > I'll look at this tomorrow and let you know. I'm looking at Fil's
>>> > branch for https://issues.apache.org/jira/browse/CB-1091 today
>>> >
>>> > On Tue, Jul 24, 2012 at 1:23 PM, Becky Gibson <gi...@gmail.com>
>>> > wrote:
>>> > > I think I have the ARC conversion completed.   I can pass all
>>> mobile-spec
>>> > > automated tests on my iPhone 4S and iPad 2 - both running 5.1.1. The
>>> > > mobile-spec manual tests work, also.  I've also done some testing with
>>> my
>>> > > own files.   There is still some cleanup needed as I commented many
>>> > things
>>> > > rather than removing them so people could at least get some idea of the
>>> > > changes.   Any property that was marked as retain is now strong.   I
>>> > > generally left the default for local vars as strong.  This is because
>>> > since
>>> > > we are still supporting 4.2 we can't use the weak keyword and must use
>>> > > __unsafe__unretained which looks pretty scary in the code (although
>>> maybe
>>> > > it is a good idea to be reminded of those pointers that will not get
>>> set
>>> > to
>>> > > nil and may dangle).
>>> > >
>>> > > I didn't issue a pull request yet but would appreciate it if anyone has
>>> > > time to give it a look through.  We may want to wait a few more days
>>> > before
>>> > > merging but if we want to get this in for 2.1 we should probably do it
>>> by
>>> > > next week.   Also, I should probably update a few plugins to ARC as
>>> well
>>> > -
>>> > > any suggestions for which ones?   Also, there are two commits in my
>>> > branch
>>> > > as I forgot to do a --force on one of my updates.  I guess I can fix
>>> that
>>> > > with git rebase --i but it isn't one of my favorite commands if there
>>> are
>>> > > other methods to squash those together.
>>> > >
>>> > > The CDVUIViewController,  CDVLocalStorage and CDVFileTransfer had lots
>>> of
>>> > > changes during the last month that I had to merge so if any additional
>>> > > testing of those would be appreciated.  I don't have very thorough test
>>> > > files for storage and file transfer.  Contacts has the most changes
>>> since
>>> > > it has to bridge to the core foundation classes.  I did fairly thorough
>>> > > testing of contacts but it is such a large api it is hard to test
>>> > > everything.    I didn't attempt to transition JSONKit.* to ARC, it has
>>> > > the -fno-objc-arc complier flag set.   I modified the app project
>>> > settings
>>> > > for ARC in addition to the CordovaLib settings.
>>> > >
>>> > >
>>> > > thanks,
>>> > > -becky
>>> >
>>>

Re: Update on iOS ARC conversion

[Becky Gibson <gi...@gmail.com>]

Great, thanks!   I'll merge with the lastest code and try to commit on
Thursday morning.

-becky


On Wed, Aug 1, 2012 at 4:26 PM, Shazron <sh...@gmail.com> wrote:

> Using your ARC branch, I've verified that mobile-spec tests pass and
> the relevant obj-c unit tests as well. I think this is good to go for
> a merge.
>
> On Tue, Jul 31, 2012 at 2:34 PM, Shazron <sh...@gmail.com> wrote:
> > Thanks Becky,
> > I'll take a look later today or tomorrow morning. Ideally I want the
> > plugin upgrade guide in the main docs repo, but until then we should
> > put it in the Plugin Upgrade Guide with a note in the Upgrading Guide
> > to check it out.
> >
> > On Tue, Jul 31, 2012 at 2:18 PM, Becky Gibson <gi...@gmail.com>
> wrote:
> >> I've merged in all of the latest iOS commits into my branch.  I updated
> my
> >> commits to remove the commented out code.  Fixed some ARC-conversion
> memory
> >> crashes in Contacts (the bridging to core foundation classes can be
> >> tedious).  All of the mobile-spec tests run on the iPhone Simulator and
> my
> >> iPhone 4S.  I also ran my own set of tests.
> >>
> >> I'd love to check this in before there are more commits to trunk that I
> >> have to merge.  It would be great if someone could pull down this code
> and
> >> do a make and create a new project to validate my testing.  I am still
> >> running Lion and xcode 4.3.2.
> >>
> >> What type of document do we need to explain this?  I guess it is just a
> >> matter of telling people how to add compile flags to individual plugin
> >> files if they want to use non-ARC code with the ARC enabled Cordova code
> >> and *.xcodeproj files.   Does this go in the plugin upgrade guide?
> >>
> >> thanks,
> >> -becky
> >>
> >>
> >> On Wed, Jul 25, 2012 at 3:23 PM, Andrew Grieve <ag...@chromium.org>
> wrote:
> >>
> >>> I had a look through your diff. It's nice to see all the
> >>> retain/release/autoreleases gone, but a bit scary at the same time :P.
> I
> >>> think you're probably fine to delete the commented out parts, and it
> will
> >>> probably make the diff even easier to read (fewer lines in the diff
> that
> >>> way).
> >>>
> >>> Just from the limited work I've done on FileTransfer, I know that the
> >>> mobile-spec tests (automated ones) for it give pretty good coverage.
> >>>
> >>> Agree that it'd be good to get this in soon so we can maximize testing
> on
> >>> it.
> >>>
> >>> Good call on leaving JSONKit as-is.
> >>>
> >>>
> >>> Andrew
> >>>
> >>>
> >>> On Tue, Jul 24, 2012 at 4:41 PM, Shazron <sh...@gmail.com> wrote:
> >>>
> >>> > Thanks Becky!
> >>> > I would start with ChildBrowser and then choose any other ones by
> last
> >>> > modified and medium complex
> >>> > https://github.com/phonegap/phonegap-plugins/tree/master/iOS (eg
> >>> > BarcodeScanner, NavigationBar).
> >>> >
> >>> > I'll look at this tomorrow and let you know. I'm looking at Fil's
> >>> > branch for https://issues.apache.org/jira/browse/CB-1091 today
> >>> >
> >>> > On Tue, Jul 24, 2012 at 1:23 PM, Becky Gibson <
> gibson.becky@gmail.com>
> >>> > wrote:
> >>> > > I think I have the ARC conversion completed.   I can pass all
> >>> mobile-spec
> >>> > > automated tests on my iPhone 4S and iPad 2 - both running 5.1.1.
> The
> >>> > > mobile-spec manual tests work, also.  I've also done some testing
> with
> >>> my
> >>> > > own files.   There is still some cleanup needed as I commented many
> >>> > things
> >>> > > rather than removing them so people could at least get some idea
> of the
> >>> > > changes.   Any property that was marked as retain is now strong.
> I
> >>> > > generally left the default for local vars as strong.  This is
> because
> >>> > since
> >>> > > we are still supporting 4.2 we can't use the weak keyword and must
> use
> >>> > > __unsafe__unretained which looks pretty scary in the code (although
> >>> maybe
> >>> > > it is a good idea to be reminded of those pointers that will not
> get
> >>> set
> >>> > to
> >>> > > nil and may dangle).
> >>> > >
> >>> > > I didn't issue a pull request yet but would appreciate it if
> anyone has
> >>> > > time to give it a look through.  We may want to wait a few more
> days
> >>> > before
> >>> > > merging but if we want to get this in for 2.1 we should probably
> do it
> >>> by
> >>> > > next week.   Also, I should probably update a few plugins to ARC as
> >>> well
> >>> > -
> >>> > > any suggestions for which ones?   Also, there are two commits in my
> >>> > branch
> >>> > > as I forgot to do a --force on one of my updates.  I guess I can
> fix
> >>> that
> >>> > > with git rebase --i but it isn't one of my favorite commands if
> there
> >>> are
> >>> > > other methods to squash those together.
> >>> > >
> >>> > > The CDVUIViewController,  CDVLocalStorage and CDVFileTransfer had
> lots
> >>> of
> >>> > > changes during the last month that I had to merge so if any
> additional
> >>> > > testing of those would be appreciated.  I don't have very thorough
> test
> >>> > > files for storage and file transfer.  Contacts has the most changes
> >>> since
> >>> > > it has to bridge to the core foundation classes.  I did fairly
> thorough
> >>> > > testing of contacts but it is such a large api it is hard to test
> >>> > > everything.    I didn't attempt to transition JSONKit.* to ARC, it
> has
> >>> > > the -fno-objc-arc complier flag set.   I modified the app project
> >>> > settings
> >>> > > for ARC in addition to the CordovaLib settings.
> >>> > >
> >>> > >
> >>> > > thanks,
> >>> > > -becky
> >>> >
> >>>
>