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

Remove FB copyright notices from iOS template #27725

Closed

Conversation

alloy
Copy link
Contributor

@alloy alloy commented Jan 9, 2020

Summary

Files that are to be part of the user’s project shouldn’t have FB copyright notices.

There’s one notice left in the android template, which is in a file that doesn’t seem to be intended to be changed by the user and so that seems fine to me.

Changelog

[iOS] [Removed] - Remove copyright notices from iOS application template

Test Plan

npx react-native init TestTemplateUpdates --template ~/Code/ReactNative/react-native
cd ios
xcodebuild -workspace TestTemplateUpdates.xcworkspace \
  -scheme TestTemplateUpdates \
  -destination 'platform=iOS Simulator,OS=latest,name=iPhone 8' build
[…]
** BUILD SUCCEEDED **

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 9, 2020
@@ -557,7 +556,7 @@
"-ObjC",
"-lc++",
);
PRODUCT_BUNDLE_IDENTIFIER = "com.facebook.REACT.HelloWorld-tvOS";
PRODUCT_BUNDLE_IDENTIFIER = "org.reactjs.native.example.HelloWorld-tvOS";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These PRODUCT_BUNDLE_IDENTIFIER changes are to bring the tvOS products in line with the iOS one.

@elicwhite
Copy link
Member

@cpojer / @zertosh, do you know how we can make certain source files in the repo not require the license headers?

@zertosh
Copy link
Member

zertosh commented Jan 10, 2020

@TheSavior, the template/ directory is already excluded from the linter. If you want to make sure these files don't get license header by mistake, then just add a pattern lint for "copyright" that covers these files.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@TheSavior is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@alloy
Copy link
Contributor Author

alloy commented Jan 10, 2020

@zertosh Just a heads-up, but those links don’t work for people like me that have no FB account.

@elicwhite
Copy link
Member

Yeah, he knows. They were internal fb links for config files I will need to change in other parts of our monorepo :-)

@alloy
Copy link
Contributor Author

alloy commented Jan 10, 2020

Ah ok, I see 👍

@elicwhite
Copy link
Member

This is failing our internal CI with this log:

Stderr:
xcodebuild: error: Unable to find a destination matching the provided destination specifier:
		{ platform:iOS Simulator, OS:12.4, name:iPhone 6s }
	The requested device could not be found because no available devices matched the request.
	Available destinations for the "RNTester" scheme:
		{ platform:iOS Simulator, id:BEDA284B-F1F0-4C3B-9159-E3D1978C6B4F, OS:13.3, name:iPad (5th generation) }
		{ platform:iOS Simulator, id:EC4ABE43-542F-4E02-9739-F6E0FC014A2F, OS:13.3, name:iPad (6th generation) }
		{ platform:iOS Simulator, id:B5F8BF1C-DB7C-4670-820D-DCF8CC1E640C, OS:13.3, name:iPad (7th generation) }
		{ platform:iOS Simulator, id:A8FA7A65-F24F-46C6-8E74-496333886DCF, OS:13.3, name:iPad Air (3rd generation) }
		{ platform:iOS Simulator, id:FA38D4BF-3E83-465C-A2A6-FF68F03A46A8, OS:13.3, name:iPad Air 2 }
		{ platform:iOS Simulator, id:FCDCE3F0-2A21-40D6-8B5D-7B88907604D8, OS:13.3, name:iPad Pro (9.7-inch) }
		{ platform:iOS Simulator, id:6A33B87D-C9B5-4DF7-B69B-7BE7B8660CAC, OS:13.3, name:iPad Pro (10.5-inch) }
		{ platform:iOS Simulator, id:A13E2893-0070-48E4-8C20-784042747447, OS:13.3, name:iPad Pro (11-inch) }
		{ platform:iOS Simulator, id:CA30731D-DF0D-4567-A83D-808190ABD1C3, OS:13.3, name:iPad Pro (12.9-inch) }
		{ platform:iOS Simulator, id:02738396-E246-4690-B1B5-D769F1168054, OS:13.3, name:iPad Pro (12.9-inch) (2nd generation) }
		{ platform:iOS Simulator, id:D28C6174-3F66-4D84-8597-390A3BF61282, OS:13.3, name:iPad Pro (12.9-inch) (3rd generation) }
		{ platform:iOS Simulator, id:1F1639E6-2F63-45E0-93AC-68CE0AE2EE6B, OS:13.3, name:iPad mini (5th generation) }
		{ platform:iOS Simulator, id:169A834D-E7E0-4BFF-A77E-39BDF5F93577, OS:13.3, name:iPad mini 4 }
		{ platform:iOS Simulator, id:170B1951-2338-4FC1-A671-FB75A01F958C, OS:13.3, name:iPhone 6s }
		{ platform:iOS Simulator, id:4D182D7B-71AA-44D9-AFFA-BF854A4700EF, OS:13.3, name:iPhone 6s Plus }
		{ platform:iOS Simulator, id:7BA3D987-E721-453B-AB07-8BDA3CF52674, OS:13.3, name:iPhone 7 }
		{ platform:iOS Simulator, id:DADB09CE-EFD9-4482-8F8D-10399D70B1EA, OS:13.3, name:iPhone 7 Plus }
		{ platform:iOS Simulator, id:981CBD9A-452E-4F04-AB95-EC65338AF116, OS:13.3, name:iPhone 8 }
		{ platform:iOS Simulator, id:59BF77F0-2041-4B75-9EB0-270F3D177352, OS:13.3, name:iPhone 8 Plus }
		{ platform:iOS Simulator, id:2CBFCAE7-FC40-4F8C-AF85-E5FAF5060888, OS:13.3, name:iPhone 11 }
		{ platform:iOS Simulator, id:C11D9D20-F163-46F5-A912-BFC38C83CAB3, OS:13.3, name:iPhone 11 Pro }
		{ platform:iOS Simulator, id:3C6F0440-2E30-49E6-8121-6023FB6C0BE4, OS:13.3, name:iPhone 11 Pro Max }
		{ platform:iOS Simulator, id:ADBCE772-FECA-4498-AEB5-9820D18013DC, OS:13.3, name:iPhone SE }
		{ platform:iOS Simulator, id:9362811F-B0DB-4F28-9E48-CCF590FC41B6, OS:13.3, name:iPhone X }
		{ platform:iOS Simulator, id:9382CA28-147D-4997-A87F-9CDEC10240E2, OS:13.3, name:iPhone Xs }
		{ platform:iOS Simulator, id:55558607-EA76-4C53-8283-EF5EC88FF340, OS:13.3, name:iPhone Xs Max }
		{ platform:iOS Simulator, id:AE262A3C-17AB-49B2-87D4-B8A60C6EC58F, OS:13.3, name:iPhone Xʀ }

I believe @hramos has worked on upgrading the ios simulator internally and may have context on this.

@hramos
Copy link
Contributor

hramos commented Jan 14, 2020

@rickhanlonii's 5cd0c8a should fix the simulator issue that @TheSavior raised. I also have https://our.internmc.facebook.com/intern/diff/D18849584/ (mirrored to GitHub in https://github.com/hramos/react-native/tree/export-D18849584) in flight which should bring tests to Xcode 11.2.1, with Xcode 11.3 coming soon after.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @alloy in 9c3fa57.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jan 23, 2020
@elicwhite
Copy link
Member

It landed!

@alloy alloy deleted the remove-fb-copyright-notices-from-templates branch January 27, 2020 16:26
alloy added a commit to alloy/react-native that referenced this pull request Feb 5, 2020
Summary:
Files that are to be part of the user’s project shouldn’t have FB copyright notices.

There’s [one notice left](https://github.com/facebook/react-native/blob/66601e755fcad10698e61d20878d52194ad0e90c/template/android/app/src/debug/java/com/helloworld/ReactNativeFlipper.java#L2) in the `android` template, which is in a file that doesn’t seem to be intended to be changed by the user and so that seems fine to me.

[iOS] [Removed] - Remove copyright notices from iOS application template
Pull Request resolved: facebook#27725

Test Plan:
```bash
npx react-native init TestTemplateUpdates --template ~/Code/ReactNative/react-native
cd ios
xcodebuild -workspace TestTemplateUpdates.xcworkspace \
  -scheme TestTemplateUpdates \
  -destination 'platform=iOS Simulator,OS=latest,name=iPhone 8' build
[…]
** BUILD SUCCEEDED **
```

Differential Revision: D19343386

Pulled By: TheSavior

fbshipit-source-id: a78be5e5d2fdc2477adedb51d6bb3ff19845b75f
osdnk pushed a commit to osdnk/react-native that referenced this pull request Mar 9, 2020
Summary:
Files that are to be part of the user’s project shouldn’t have FB copyright notices.

There’s [one notice left](https://github.com/facebook/react-native/blob/66601e755fcad10698e61d20878d52194ad0e90c/template/android/app/src/debug/java/com/helloworld/ReactNativeFlipper.java#L2) in the `android` template, which is in a file that doesn’t seem to be intended to be changed by the user and so that seems fine to me.

## Changelog

[iOS] [Removed] - Remove copyright notices from iOS application template
Pull Request resolved: facebook#27725

Test Plan:
```bash
npx react-native init TestTemplateUpdates --template ~/Code/ReactNative/react-native
cd ios
xcodebuild -workspace TestTemplateUpdates.xcworkspace \
  -scheme TestTemplateUpdates \
  -destination 'platform=iOS Simulator,OS=latest,name=iPhone 8' build
[…]
** BUILD SUCCEEDED **
```

Differential Revision: D19343386

Pulled By: TheSavior

fbshipit-source-id: a78be5e5d2fdc2477adedb51d6bb3ff19845b75f
alloy added a commit to microsoft/react-native-macos that referenced this pull request Apr 2, 2020
tom-un pushed a commit to microsoft/react-native-macos that referenced this pull request Apr 2, 2020
* [macos-init] Build on publish

* [generate-macos] Cleanup copy-paste leftover

* [generator-macos] Remove FB copyright notices

Equal to upstream iOS change facebook#27725

* [generator-macos] Name app product without `-macOS` suffix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications. Type: Removal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants