Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Post Install Concept #101

Merged
merged 4 commits into from
Aug 12, 2016
Merged

Post Install Concept #101

merged 4 commits into from
Aug 12, 2016

Conversation

sean-perkins
Copy link
Collaborator

Current Situation:
In a multi-developer environment, each developer has to manually configure their Podfile and include.gradle file to include any of the add-on micro-services that firebase offers. This isn't ideal, because often times users are not aware of these steps when being added to an existing project.

Proposed Solution:
This pull-request contains a post-install script that will run after every installation of the firebase plugin. It prompts the user if they are using iOS and/or Android and then walks the user through the available micro-services. Once the prompts complete, the script will overwrite the Podfile and include.gradle file with the base dependencies, as well as the selected micro-services. Items that they prompted "n" (no) for, will be commented out as they were previously.

I'm open feedback and/or changes. I just ran into this issue with my own plugins and found that this was an effective solution for my team environment.

Concept:

if (err) {
return console.log(err);
}
if (result.using_android && result.using_android.toLowerCase() === 'y') {
Copy link
Owner

Choose a reason for hiding this comment

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

You could use the isSelected convenience function here as well.

@EddyVerbruggen
Copy link
Owner

Man, I totally love it! ❤️

I only have a few trivial remarks but without those I think it's already good enough to merge, so your call if you want to add anything before I merge it.

  • Can you add a message at the end of the script telling folks how to re-run the script if they made a mistake or want to add a microservice at a later time?
  • Can you change the 'Are you using Facebook Authentication' prompt to 'Are you using Firebase Facebook Authentication'? I totally misread that line as 'Are you using Firebase Authentication' since it visually aligns so well with the previous lines :)
    screen shot 2016-08-11 at 11 58 38
  • Same for Google.

Btw, please don't remove the Podfile and include.gradle files for now (which you haven't, so that's cool) because I'm quite sure it will break builds for Telerik Platform users. I can live with the duplicate maintenance for now.

Also, seeing this work so well do you think it would be possible to automate adding those 2 lines to the platforms/android/build.gradle script as a post install step? That would require the Android platform to be already added, but that's probably mostly the case.

I will grant you commit access so you can work in this repo directly if you please (fi. for hotfixes to the script, if required).

@ullalaaron
Copy link

This is Amazing!

@EddyVerbruggen EddyVerbruggen mentioned this pull request Aug 11, 2016
@sean-perkins
Copy link
Collaborator Author

@EddyVerbruggen Thanks for the great feedback! I'll be digging into your suggested improvements now 👍

@sean-perkins
Copy link
Collaborator Author

@EddyVerbruggen The suggested improvements have been implemented. This branch should be ready to merge.

Thanks!!

@jlooper
Copy link
Contributor

jlooper commented Aug 11, 2016

I'm so excited about this 👯 👯 👯 👯 👯

}

repositories {
mavenCentral()

Choose a reason for hiding this comment

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

With how gradle works, gradle selects the first repository that contains the required dependency and uses that. per this release Android Studio migrated their default repository to jcenter.

jcenter is a superset of mavenCentral with the added bonus of serving their content over HTTPS and a CDN!

My recommendation would be to place jcenter first so at least the repositories will default to this preference. The more extreme being that since it's a superset, removing mavenCentral shouldn't have any issues.

@EddyVerbruggen
Copy link
Owner

EddyVerbruggen commented Aug 11, 2016

Great, Great work!

I'll merge this tomorrow, so if anyone want to add (more) comments go right ahead.

@sean-perkins The README and the files in /docs will need adjustment but I'll be more than happy to do that myself after this is merged.

@white0ut, also thanks for your suggestion, I'll be keeping that in mind.

@EddyVerbruggen
Copy link
Owner

EddyVerbruggen commented Aug 12, 2016

Hey @sean-perkins do you agree we can remove the 'application package' step since NativeScript added it (recently) by default to in 2.1.0. It gets written to app/App_Resources/Android/app.gradle in the exact same spot of the gradle hierarchy as the postinstall script would.

I'll also remove that manual step from the readme and urge users to use 2.1.0+.

@vekexasia
Copy link

@sean-perkins great job. I was in the process of doing something similar. Thank god I ended up here.

@EddyVerbruggen when do you think it will be merged?

@EddyVerbruggen
Copy link
Owner

@vekexasia today

@sean-perkins
Copy link
Collaborator Author

@EddyVerbruggen That sounds like a great plan to promote the later builds. I will commit the removal in a second.

@EddyVerbruggen EddyVerbruggen merged commit fbda427 into EddyVerbruggen:master Aug 12, 2016
@EddyVerbruggen
Copy link
Owner

Merged! 🎉

I'll tweak the docs where needed.

EddyVerbruggen added a commit that referenced this pull request Aug 12, 2016
- trivial cleanup
- doc adjustments
- version bump
@jlooper
Copy link
Contributor

jlooper commented Aug 16, 2016

@EddyVerbruggen is this documented? @sean can we make a lovely NativeScriptSnack of your video? wonderful work!

@EddyVerbruggen
Copy link
Owner

Yes it is

@sean-perkins
Copy link
Collaborator Author

@jlooper Do you want a write-up of the process and concept of post-install or specifically the firebase post-install process for this plugin?

@jlooper
Copy link
Contributor

jlooper commented Aug 16, 2016

@sean I was just thinking that a video of the exact process of the post-install process for this plugin would be really good to include on the snacks site. In terms of docs, I was looking for it on the homepage of the readme but maybe missed it.

EddyVerbruggen added a commit that referenced this pull request Apr 18, 2017
EddyVerbruggen added a commit that referenced this pull request Apr 18, 2017
- trivial cleanup
- doc adjustments
- version bump
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants