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(iOS): Refactoring configuration #3759

Merged
merged 23 commits into from
Nov 24, 2020
Merged

feat(iOS): Refactoring configuration #3759

merged 23 commits into from
Nov 24, 2020

Conversation

ikeith
Copy link
Contributor

@ikeith ikeith commented Nov 3, 2020

This branch revisits configuration with multiple goals:

  • Move the configuration store away from a key/value container to typed properties. This reduces the risk of "stringly-typed" parsing scattered around the project and improves type safety.

  • Centralize all configuration defaults and file keys so they are not spread over the codebase. This also allows for easier testing and updating of the configuration file.

  • Provide a programmatic structure so applications can configure the capacitor bridge without relying on injecting JSON.

  • Add unit tests.

The configuration file(s) is parsed into an intermediate structure which can be modified (supported in a future refactoring of CAPBridgeViewController), and then transformed into an immutable store on the bridge.

CAPLog has been updated to use a simple boolean to control logging instead of loading a second instance of CAPConfig (and making assumptions about its location). The initialization has been reworked to set that flag based on the configuration before any logging might occur.

@ikeith ikeith changed the title Configuration chore(iOS): Updating configuration Nov 5, 2020
@ikeith ikeith marked this pull request as ready for review November 5, 2020 18:39
@ikeith ikeith added this to In progress 🤺 in Capacitor Engineering ⚡️ via automation Nov 6, 2020
@ikeith ikeith added this to the 3.0.0 milestone Nov 6, 2020
@ikeith ikeith moved this from In progress 🤺 to Needs review 🤔 in Capacitor Engineering ⚡️ Nov 6, 2020
@ikeith ikeith changed the title chore(iOS): Updating configuration feat(iOS): Refactoring configuration Nov 6, 2020
Copy link
Contributor

@imhoffd imhoffd left a comment

Choose a reason for hiding this comment

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

  • I found @class CAPConfig; in CAPPlugin.h -- is this a problem?

for (itemIndex, item) in items.enumerated() {
Swift.print(item, terminator: itemIndex == items.count - 1 ? terminator : separator)
}
}
}

public static func hideLogs() -> Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this was public to begin with, but just noting that it is something being removed from the public API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, its replacement property is public as well but we should probably note that.

ios/Capacitor/Capacitor/KeyPath.swift Show resolved Hide resolved
ios/Capacitor/Capacitor/KeyPath.swift Show resolved Hide resolved
@ikeith
Copy link
Contributor Author

ikeith commented Nov 20, 2020

  • I found @class CAPConfig; in CAPPlugin.h -- is this a problem?

Good catch. It's an unused forward declaration so, no, it's not a problem but it should be removed anyway.

@ikeith ikeith merged commit e2e64c2 into main Nov 24, 2020
Capacitor Engineering ⚡️ automation moved this from Needs review 🤔 to Done 🎉 Nov 24, 2020
@ikeith ikeith deleted the configuration branch November 24, 2020 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants