zeplin / fastlane-plugin-notarize

fastlane plugin to notarize a macOS app πŸ›‚
MIT License
68 stars 8 forks source link

πŸ‘‹ Thoughts on pull requesting this into a core fastlane action? #22

Open joshdholtz opened 4 years ago

joshdholtz commented 4 years ago

πŸ‘‹ 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 commented 4 years ago

Hey @joshdholtz πŸ‘‹

It’d 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 commented 4 years ago

@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 commented 4 years ago

Awesome! πŸ™Œ @berkcebi just opened the PR. πŸ₯³

When the next fastlane version is released, I’m 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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Hey @joshdholtz ,

Thanks again for reaching out and working with us on this. πŸ™Œ I’ve 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’t 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 commented 4 years ago

@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! ❀️