[NO-TKT] Dependency Updates #39
Conversation
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ This pull request was sent to the PullRequest network.
@louisvdaily you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
This looks straightforward and presumably if CI passes, this will be (mostly 🤞️) quite reasonable. Added a small comment on the new wrapper script to perhaps make its tricky bit (the list of excluded architectures) slightly more maintainable.
Reviewers will be notified any time you reply to their comments or commit new changes.
You can also request a full follow-up review from your PullRequest dashboard.
|
|
||
| # For Xcode 12 make sure EXCLUDED_ARCHS is set to arm architectures otherwise | ||
| # the build will fail on lipo due to duplicate architectures. | ||
| echo 'EXCLUDED_ARCHS__EFFECTIVE_PLATFORM_SUFFIX_simulator__NATIVE_ARCH_64_BIT_x86_64__XCODE_1200 = arm64 arm64e armv7 armv7s armv6 armv8' >> $xcconfig |
There was a problem hiding this comment.
Is there a way to generate the list of architectures here instead of hardcoding it (or at least can you move it to a "constant" variable defined at the top of the file?
Given this needs to be an explicit exclusion list I presume it would break if a new architecture were to be added, which might lead to future maintenance overhead.
There was a problem hiding this comment.
Based on the description, this implementation looks sound, and straight forward. I have reviewed the code and I don't see any outstanding issue.
Reviewers will be notified any time you reply to their comments or commit new changes.
You can also request a full follow-up review from your PullRequest dashboard.
louisvdaily
force-pushed
the
dependency-updates
branch
from
3dcfcc9 to
0ec06e2
Compare
There was a problem hiding this comment.
⚠️ Warning
PullRequest detected a force-push on this branch. This may have caused some information to be lost, and additional time may be required to complete review of the code. Read More
…h script in bootstrap script.
louisvdaily
force-pushed
the
dependency-updates
branch
from
0ec06e2 to
af33d71
Compare
Codecov Report
@@ Coverage Diff @@
## master #39 +/- ##
=======================================
Coverage 95.08% 95.08%
=======================================
Files 35 35
Lines 712 712
=======================================
Hits 677 677
Misses 35 35 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Due to inactivity, PullRequest has cancelled this review job. You can reactivate the code review job from the PullRequest dashboard.
1 participant
Purpose
Associated Tickets
Details
This is a thing that does stuff for widgets.
Checklist