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

Added two more apps #79

Closed
wants to merge 1 commit into from
Closed

Added two more apps #79

wants to merge 1 commit into from

Conversation

Maryom
Copy link
Contributor

@Maryom Maryom commented Sep 20, 2016

Salaam,

I added two more apps so that total number will be 155. I need to edit readme as well but I'm little bit lost because there are two branches. Ok I will do it إن شاء الله when I know how! I'm searching

One small note:
I think we should delete -> Bool from👇🏼

public func open<E: ExternalApplication>(_ externalApp: E, action: E.ActionType, promptInstall: Bool = false) -> Bool {

And we should delete it here as well:

func openURL(_ url: URL) -> Bool

Because it's really annoying when developer get this warning and then silent it with let _

Result of call to 'open(_:action:promptInstall:)' is unused

I got the above warning for each app when I wrote this for example:

apps.open(Applications.Skitch(), action: .open)

Do you agree with me?

Salaam,

I added two more apps so that total number will be 155.
@Mazyod
Copy link
Member

Mazyod commented Sep 20, 2016

Sorry about the delay with Swift 3. I haven't merged it in the past because there was a bug with Cocoapods, so I couldn't update the podspec. I'll check if the bug has been resolved.

Regarding the comment about the warning, I fixed it using @discardableResult. It will no longer show warnings for you.

Once I merge master, you can submit the PR there.

@Maryom
Copy link
Contributor Author

Maryom commented Sep 20, 2016

It's totally fine :) should I close this PR ? Can I help with the Cocoapods bug?

@Maryom
Copy link
Contributor Author

Maryom commented Sep 20, 2016

By the way I found this helpful links:
https://drafts4-actions.agiletortoise.com/search?button=&page=49&q=&search_type=apps&utf8=✓

إن شاء الله some day I will try to write script to help us to add all these apps.

@Mazyod
Copy link
Member

Mazyod commented Sep 20, 2016

Wow, that website is perfect.. I think we should change the framework design a bit before you start writing that script. Adding a swift file per app is not very scalable, and will cause compilation issues and excessive bloat .. So we will need to figure out a way to simplify the framework first.

@Maryom
Copy link
Contributor Author

Maryom commented Sep 20, 2016

Ok deal 👍🏼 when you want us to start changing the framework design please could you tell me the suggestions you prefer to do them?

@Mazyod
Copy link
Member

Mazyod commented Sep 20, 2016

I really don't know yet ... We can discuss possible ideas here

@Mazyod Mazyod closed this Sep 20, 2016
@Mazyod
Copy link
Member

Mazyod commented Sep 20, 2016

Somehow, I managed to push the new version to Cocoapods. You can now submit the PR again against master branch, if you like.

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.

2 participants