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

Add option to add plist to target or not #43

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

JustinFincher
Copy link

@JustinFincher JustinFincher commented Apr 30, 2017

When I uses this plugin, I encountered a situation where the plist bundled in settings.bundle is better not added to target’s Copy Bundle Resources process.
So I made some changes to lib/cocoapods_acknowledgements.rb, and by settings :settings_bundle => true, :add_to_target => false, plist will not be automatically added to Copy Bundle Resources process.

@orta
Copy link
Member

orta commented Apr 30, 2017

👍 Looks good, yeah, can you please add some tests and a CHANGELOG, then this is good to go - thanks!

@JustinFincher
Copy link
Author

JustinFincher commented May 2, 2017

Hi, when I was doing local tests for my own project, I found that the new self.settings_bundle_in_project in the master branch won't return the full relative path.
For example, my own project has the following structure:

├── Cetacea
│   ├── Cetacea-iOS
│   │   ├── Cetacea-iOS
│   │   │   └── Settings.bundle
│   │   └── Cetacea-iOS.xcodeproj
│   ├── Cetacea-Mac
│   │   └── Cetacea-Mac.xcodeproj
│   ├── CetaceaSharedFramework
│   │   └── CetaceaSharedFramework.xcodeproj
│   ├── Cetacea.xcworkspace
│   ├── Podfile
│   ├── Pods
│   │   ├── Pods.xcodeproj

When I am using the 6469aeb (tag 1.1.2), I got:

settings_bundle = Cetacea-iOS/Cetacea-iOS/Settings.bundle

When I am using the eadb277 commit, I got:

settings_bundle = Cetacea-iOS/Settings.bundle

I got no file or folder error:

Errno::ENOENT - No such file or directory @ rb_sysopen

My guess is that cocoapods's working folder is the root Cetacea folder, and when using the newest commit, file = project.files.find { |f| f.path =~ /Settings\.bundle$/ } returns the relative path from the project, not the root folder, so the path can't be accessed.
So I revert back to the old self.settings_bundle_in_project method in SecondRenaissance@eb5bc7b.

@imnosov
Copy link

imnosov commented Sep 27, 2017

@orta Could you please merge this pull request?

@orta
Copy link
Member

orta commented Sep 27, 2017

I wasn't sure if @JustinFincher was finished with it? happy to do so if he is

@JustinFincher
Copy link
Author

JustinFincher commented Sep 27, 2017

The basic functionality is done. but I haven’t done the tests and changelog part because late on I abandoned my project 🤣.
I will try to write some tests when I get home, and remove the install bash script as that script is only for my convenient use.

@JustinFincher
Copy link
Author

@orta Hi, can you give me some examples about how to test this plugin? Because I found the repo seems doesn't have test files or patterns for me to follow. Should I provide a sample iOS project?

@orta
Copy link
Member

orta commented Sep 27, 2017

I assume that I used to run it against my iOS projects, the no-tests is a shame yeah - you're welcome to add an example iOS app to work against

@kdawgwilk
Copy link

I just gave this branch a go on our project when master was failing to add the plist files with the correct path to the Xcode project and this fixed our issues 👍 Maybe that is the testing you were needing?

@kdawgwilk
Copy link

Sorry I spoke too soon :( It appeared to fix the issue but after cleaning things up and running fresh I still had the same issues

@jparise
Copy link

jparise commented May 1, 2023

I just proposed #65, which does something similar (makes project integration optional) by handling the installer's integrate_targets: false option.

If it's accepted, my change could further be extended with a plugin-level option to explicitly enable this behavior, which would match this PR, although the implicit behavior might be enough to handle most cases on its own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants