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

feat(cli): support capacitor portals plugin changes needed #5558

Merged
merged 14 commits into from
Apr 21, 2022
Merged

Conversation

carlpoole
Copy link
Member

No description provided.

@carlpoole carlpoole self-assigned this Apr 8, 2022
cli/src/tasks/copy.ts Outdated Show resolved Hide resolved
@carlpoole carlpoole requested a review from IT-MikeS April 11, 2022 16:27
cli/src/tasks/copy.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@IT-MikeS IT-MikeS left a comment

Choose a reason for hiding this comment

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

This looks good to me, nice and clean 👍

@jthoms1
Copy link
Contributor

jthoms1 commented Apr 20, 2022

@jcesarmobile Brought up the point that we might use hooks from plugins rather than having the copy code in CLI core. We discussed this as a group and feel like that might cause more issues moving forward.

Hooks for platforms makes more sense because the code mods are only controlled by a single codebase. Electron/iOS/Android.

But in the case of allowing for code mods or hooks from Plugins we run into issues. These hooks can introduce unpredictable side effects. Where the order of plugin loading can yield different results and individual plugins can cause major issues with projects without devs realizing this in the usage.

For this case we believe it is best to continue with this simplistic approach of having the code mods in the CLI core codebase.

@jcesarmobile jcesarmobile removed their request for review April 20, 2022 17:30
@carlpoole carlpoole marked this pull request as ready for review April 21, 2022 18:45
@carlpoole carlpoole changed the title feat(cli): support federated capacitor plugin changes needed feat(cli): support capacitor portals plugin changes needed Apr 21, 2022
@carlpoole carlpoole merged commit 6810a19 into main Apr 21, 2022
@carlpoole carlpoole deleted the cli-fedcap branch April 21, 2022 20:52
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.

4 participants