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.

Sign up for GitHub

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

Jump to bottom

Fix various build-time warnings #trivial #3938

Merged
merged 2 commits into from
Sep 29, 2020
Merged

Fix various build-time warnings #trivial #3938

merged 2 commits into from
Sep 29, 2020

Conversation

ashfurrow
Copy link
Contributor

@ashfurrow ashfurrow commented Sep 28, 2020 •
edited by jira bot
Loading

The type of this PR is: upkeep ✨

Description

Xcode was showing various warnings, about unused unimplemented functions, and a type warning, that I cleaned up. I also fixed the annoying warnings after running pod install.

Before:

Screen Shot 2020-09-28 at 16 52 38

After:

Screen Shot 2020-09-28 at 16 53 16

I added a note explaining the issue to the Podfile.

PR Checklist (tick all before merging)

  • I have included screenshots or videos to illustrate my changes, or I have not changed anything that impacts the UI.
  • I have added tests for my changes, or my changes don't require testing, or I have included a link to a separate Jira ticket covering the tests.
  • I have documented any follow-up work that this PR will require, or it does not require any.
  • I have added an app state migration, or my changes do not require one. (What are migrations?)

@ashfurrow ashfurrow changed the title Fix various build-time warnings Fix various build-time warnings #trivial Sep 28, 2020
pvinis
pvinis reviewed Sep 29, 2020
View reviewed changes
Podfile
@@ -118,6 +118,10 @@ target 'Artsy' do
pod 'RNReactNativeHapticFeedback', path: 'node_modules/react-native-haptic-feedback'
pod 'react-native-webview', path: 'node_modules/react-native-webview'

# This is a dependency of RNImageCropPicker but is introducing duplicated UUIDs. We're using a previous
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a good candidate for HACKS.md? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah good call! I'll add it on master, along with a note about the PR I just opened to solve the underlying issue 🙈 TimOliver/TOCropViewController#432

brainbicycle
brainbicycle approved these changes Sep 29, 2020
View reviewed changes
Copy link
Contributor

@brainbicycle brainbicycle left a comment

Choose a reason for hiding this comment

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

Nice ✨

@MounirDhahri MounirDhahri merged commit 1db4979 into master Sep 29, 2020
@MounirDhahri MounirDhahri deleted the fix-warnings branch September 29, 2020 16:13
@ashfurrow ashfurrow mentioned this pull request Sep 30, 2020
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pvinis pvinis pvinis left review comments

@MounirDhahri MounirDhahri MounirDhahri approved these changes

@brainbicycle brainbicycle brainbicycle approved these changes

Assignees

@brainbicycle brainbicycle

Labels
Jira Synced
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashfurrow @pvinis @MounirDhahri @brainbicycle