Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

馃憢 Thoughts on pull requesting this into a core fastlane action? #22

Open
joshdholtz opened this issue Jan 29, 2020 · 7 comments
Open
Assignees

Comments

@joshdholtz
Copy link

馃憢 from the fastlane team! I started working on a built in action for fastlane called notarize but then got word that you have already created a plugin for this 馃槉

As of version 2.141.0, which was released today, fastlane now supports building and packaging of macOS and Catalyst apps and it feels like having notarize built into fastlane would be very nice feeling. I was curious what your thoughts were on pull requesting this action into the core fastlane product 馃槆

鉂わ笍

@yigitcanyurtsever yigitcanyurtsever self-assigned this Jan 30, 2020
@yigitcanyurtsever
Copy link
Member

Hey @joshdholtz 馃憢

It鈥檇 actually be great. 馃檶 We were hoping to integrate this plugin to the core product as this will become mandatory for all non-App Store macOS apps real soon. We actually even tried to add it to the core when we were first building it. 馃榿

I quickly checked the the contributing file and your first pr documents on the repo. Is there a guide or a format we can follow for adding new actions in addition to these documents?

@joshdholtz
Copy link
Author

@yigitcanyurtsever Perfect, this makes me so happy! All you need to do is paste https://github.com/zeplin/fastlane-plugin-notarize/blob/master/lib/fastlane/plugin/notarize/actions/notarize_action.rb into https://github.com/fastlane/fastlane/tree/master/fastlane/lib/fastlane/actions and then open a PR and that should be it!

I can take care of making sure it runs and adding tests and everything once you make the PR 馃挭

@yigitcanyurtsever
Copy link
Member

Awesome! 馃檶 @berkcebi just opened the PR. 馃コ

When the next fastlane version is released, I鈥檓 planning to update README file and release one final version for the plugin with a UI message suggesting to use the action on the core product. Does this make sense? Or is there any other common practice for this?

@joshdholtz
Copy link
Author

Thank you! I will get that merged in tomorrow and we will probably have a new release out early next week 馃挭 I tend to not like to do releases before weekends 馃槆

Best practice would be to add deprecation notes (see this example of the hockey action) - https://github.com/fastlane/fastlane/blob/master/fastlane/lib/fastlane/actions/hockey.rb#L393-L402

This will show a deprecation message to the user when running. The message could inform users to remove the plugin from their Pluginfile and that should be pretty good!

@yigitcanyurtsever
Copy link
Member

Thanks a lot @joshdholtz! That's the exact thing I was looking for. 馃檶 I'll try to prepare the release this week as well.

@yigitcanyurtsever
Copy link
Member

Hey @joshdholtz ,

Thanks again for reaching out and working with us on this. 馃檶 I鈥檝e released a final version for the plugin with the deprecated notes and updated the README.md.

Not sure if this is the right place but wanted to mention it before I forget: We wanted to add error handling for the sh action calls via error_callback parameter to display the actual result to the users if the action failed but didn鈥檛 find the time to do so. We'd be happy to contribute this addition later on but wanted to let you know anyways.

Cheers!

@joshdholtz
Copy link
Author

@yigitcanyurtsever Hey! If you would like to contribute that in that would be 馃挴 Otherwise feel free to file an issue and one of us will get to it sometime 馃槉

Thanks again for the code donation! 鉂わ笍

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

No branches or pull requests

2 participants