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

fix: resolve all project warnings #10830

Merged
merged 26 commits into from Sep 4, 2019
Merged

fix: resolve all project warnings #10830

merged 26 commits into from Sep 4, 2019

Conversation

hansemannn
Copy link
Collaborator

@hansemannn hansemannn commented Apr 6, 2019

JIRA: https://jira.appcelerator.org/browse/TIMOB-27163

This pull request adds full Swift 5 compatibility, in summary:

  • Update iOS example Swift module to Swift 5
  • Update iOS Swift module template to Swift 5
  • Resolve all project warnings (including missing method selectors, nullability flags and strict prototypes). Not removing the deprecations for now, which should be done in a separate ticket

Tested and confirmed to work. Common steps:

  • Create a new app and build it
  • Create a new iOS Swift module and build + import it
  • Open the Titanium Xcode project template under iphone/iphone/Titanium.xcodeproj and build it
  • The number of warnings should be reduced compared to building without this PR. Note that [TIMOB-26038] Use JavaScriptCore framework API to implement proxies #10381 introduced a lot of new deprecation warnings so the total number of warning is still pretty high.

@build
Copy link
Contributor

build commented Apr 6, 2019

Fails
🚫

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code, but will require an admin to merge this PR. Please see README.md#unit-tests for docs on unit testing.

🚫

😥 npm test failed. See below for details.

Warnings
⚠️

🔍 Can't find junit reports at ./junit.*.xml, skipping generating JUnit Report.

Messages
📖 🎉 Another contribution from our awesome community member, hansemannn! Thanks again for helping us make Titanium SDK better. 👍
📖
> titanium-mobile@8.1.0 test /Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10830
> npm run ios-sanity-check && grunt


> titanium-mobile@8.1.0 ios-sanity-check /Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10830
> ./build/scons check-ios-toplevel

Running "appcJs:src:lintOnly" (appcJs) task

Running "eslint:src" (eslint) task

/Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10830/iphone/cli/commands/_build.js
6410:79  warning  'out' is defined but never used  no-unused-vars

/Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10830/iphone/cli/hooks/frameworks.js
 61:5   warning  Expected catch() or return                  promise/catch-or-return
 61:34  warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
428:5   warning  Each then() should return a value or throw  promise/always-return

✖ 4 problems (0 errors, 4 warnings)


Running "checkFormat:ios" (checkFormat) task
Fatal error: Error: Formatting incorrect on "iphone/TitaniumKit/TitaniumKit/Sources/Kroll/KrollTimerManager.h", proposed changes: <?xml version='1.0'?>
<replacements xml:space='preserve' incomplete_format='false'>
<replacement offset='1153' length='1'></replacement>
</replacements>
�
npm ERR! Test failed.  See above for more details.

Generated by 🚫 dangerJS against 5db9c67

@janvennemann
Copy link
Contributor

@hansemannn i think this one has some merge errors. I see a lot of changes to unrelated files.

@build
Copy link
Contributor

build commented May 8, 2019

Warnings
⚠️

Commit 07da2e010412efe004cb54d9fb4e1305407b0df3 has a message "fix linting issue" giving 2 errors:

  • subject may not be empty
  • type may not be empty
⚠️

Commit 8d9dae36a58e57d04e8a3e58cfb44bd6c81f3a4c has a message "remove duplicate 'use strict' directive" giving 2 errors:

  • subject may not be empty
  • type may not be empty
⚠️

Commit c19af1892eb577a8468dfc6b8d00b5df87945876 has a message "remove duplicate hasProperty method declaration" giving 2 errors:

  • subject may not be empty
  • type may not be empty
Messages
📖

💾 Here's the generated SDK zipfile.

📖 🎉 Another contribution from our awesome community member, hansemannn! Thanks again for helping us make Titanium SDK better. 👍
📖

✅ All tests are passing
Nice one! All 4380 tests are passing.
(There are 472 tests skipped)

📖

🚨 This PR has one or more commits with warnings/errors for commit messages not matching our configuration. You may want to squash merge this PR and edit the message to match our conventions, or ask the original developer to modify their history.

Generated by 🚫 dangerJS against 86c5769

@sgtcoolguy sgtcoolguy modified the milestones: 8.1.0, 8.2.0 Jun 3, 2019
@janvennemann
Copy link
Contributor

I would like to split this PR up into the Swift version upgrade and fixing the warning messages. For the Swift version upgrade i created #10974. The warnings will be addressed via TIMOB-27163 by using the remaining changes of this PR.

@build build requested a review from a team June 18, 2019 14:29
@lokeshchdhry
Copy link
Contributor

@janvennemann , can you please resolve the conflicts.

@janvennemann janvennemann changed the title feat: support Swift 5, resolve all project warnings fix: resolve all project warnings Jul 1, 2019
@janvennemann
Copy link
Contributor

janvennemann commented Jul 1, 2019

@lokeshchdhry resolved the first merge conflicts. Please do the FR on #10974 first. Merging that will create new conflicts in this PR that i'll resolve again and then you can do the FR on this one.

@hansemannn
Copy link
Collaborator Author

Once this is merged we can directly tackle the next 100+ warnings :P

@keerthi1032
Copy link
Contributor

FR Passed. warning reduced.
Test Environment:
Name = Mac OS X
Version = 10.14.5
Architecture = 64bit
Node.js
Node.js Version = 10.16.2
npm Version = 6.9.0
Titanium CLI
CLI Version = 5.2.1
Titanium SDK
SDK Version = local sdk 8.2.0.v20190902191234
Device -iPhone X iOS 11,iphone xr iOS 13
Simulator -iPhone 8 iOS 12.4

@janvennemann
Copy link
Contributor

Most of the new warnings come from our own getter/setter deprecation and will be removed in 9.0.0.

The rest is either:

  • from external libraries (APSAnalytics)
  • iOS API deprecation notices that will be tackled in another ticket
  • value conversion warnings, which can usually be neglected

@ssjsamir ssjsamir merged commit cd60eb9 into tidev:master Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants