Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds cordova (Don't merge) #49

Closed
wants to merge 14 commits into from
Closed

Adds cordova (Don't merge) #49

wants to merge 14 commits into from

Conversation

matt-auckland
Copy link
Contributor

@matt-auckland matt-auckland commented Jun 26, 2018

[ ] iOS build needs to be tested before I'm happy to merge.
[x] We also need to figure out what we can safely gitignore.

don't need cordova here
@ro-savage ro-savage added this to In progress in Sprint for June 27 - July 10 Jun 27, 2018
@ro-savage
Copy link
Contributor

@matt-auckland - I wonder if a lot of the files can be added to .gitignore ?

@codecov
Copy link

codecov bot commented Jun 27, 2018

Codecov Report

Merging #49 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #49   +/-   ##
=======================================
  Coverage   84.07%   84.07%           
=======================================
  Files          13       13           
  Lines         157      157           
=======================================
  Hits          132      132           
  Misses         25       25

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ad072e...954f124. Read the comment docs.

@zanuka
Copy link
Contributor

zanuka commented Jul 2, 2018

@matt-auckland currently getting the following when building in xcode:

Will attempt to recover by breaking constraint 
<NSLayoutConstraint:0x6040002915d0 _UIButtonBarStackView:0x7ff8ec5151b0.trailing == _UIToolbarContentView:0x7ff8ec5046c0.trailing   (active)>

Make a symbolic breakpoint at UIViewAlertForUnsatisfiableConstraints to catch this in the debugger.```

@zanuka
Copy link
Contributor

zanuka commented Jul 2, 2018

also get this when API call is executed:
2018-07-03 11:19:48.421383+1200 Kauri Wallet[47870:2178752] NSURLConnection finished with error - code -1100

@zanuka
Copy link
Contributor

zanuka commented Jul 3, 2018

@matt-auckland I think using NSExceptionDomains in the info.plist will resolve the API call issue

zanuka
zanuka previously requested changes Jul 3, 2018
Copy link
Contributor

@zanuka zanuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to get the app to build properly on my iPhone 6S running (when running 11.4)

It tries to launch, you get the Cordova icon, but an exception is thrown with the following output:

2018-07-03 15:24:49.332251+1200 Kauri Wallet[2941:1189003] *** Terminating app due to uncaught exception 'NSUnknownKeyException', reason: '[<UIApplication 0x103e016a0> setValue:forUndefinedKey:]: this class is not key value coding-compliant for the key view.'
*** First throw call stack:
(0x183e16d8c 0x182fd05ec 0x183e169f0 0x18476c44c 0x18e0967e4 0x183d24b24 0x18e0951f4 0x18e09ac18 0x18de03e2c 0x18da211d8 0x18d9ef4d4 0x18e01fb1c 0x18d9eedd0 0x18d9eec6c 0x18d9edafc 0x18e68384c 0x18d9ed1ec 0x18e468ac8 0x18e5b6bf8 0x18d9ecc0c 0x18d9ec5a8 0x18d9e95e0 0x18d9e9330 0x186617470 0x18661fd6c 0x10240919c 0x1024157cc 0x18664b878 0x18664b51c 0x18664bab8 0x183dbf404 0x183dbec2c 0x183dbc79c 0x183cdcda8 0x185cc1020 0x18dcf9758 0x102006520 0x18376dfc0)
libc++abi.dylib: terminating with uncaught exception of type NSException```

## Builds

1. Perform a prod build in `/packages/kauri-wallet`, this will ensure Cordova has something to build with (it looks to `/packages/kauri-wallet/dist/kauri-wallet/` for the files to build with).
1. Run `npm run android` or `npm run ios` depending on which you want to build. If you want to build both run `npm run all`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meant to be step 2. here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be good to add a couple more steps or notes about requirements for cordova CLI globally as well as cd'ing into packages/cordova before running the build scripts...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

running just the ios script built out android as well. bug or feature 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using 1. 1. 1. in markdown is easier to edit (the markdown renderer will automatically turn it into a 1 2 3, which means they can be reordered or new items can be inserted easily)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mind blown. why have I never seen that? must be my old age kicking in

@zanuka
Copy link
Contributor

zanuka commented Jul 3, 2018

screen shot 2018-07-03 at 3 30 35 pm

@zanuka zanuka dismissed their stale review July 4, 2018 02:21

stale :: re-testing

@matt-auckland matt-auckland reopened this Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants