fix: Use correct category plugins to populate dev menu #897
Conversation
Codecov Report
@@ Coverage Diff @@
## main #897 +/- ##
==========================================
- Coverage 66.32% 64.84% -1.49%
==========================================
Files 870 862 -8
Lines 34434 34207 -227
==========================================
- Hits 22838 22181 -657
- Misses 11596 12026 +430
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| let pluginInfoItem: PluginInfoItem | ||
| if let versionable = versionable { | ||
| pluginInfoItem = PluginInfoItem( | ||
| displayName: pluginKey, | ||
| information: versionable.version | ||
| ) | ||
| } else { | ||
| pluginInfoItem = PluginInfoItem( | ||
| displayName: pluginKey, | ||
| information: DevMenuStringConstants.versionNotAvailable | ||
| ) | ||
| } | ||
| return pluginInfoItem | ||
| } |
There was a problem hiding this comment.
guard statement?
| let pluginInfoItem: PluginInfoItem | |
| if let versionable = versionable { | |
| pluginInfoItem = PluginInfoItem( | |
| displayName: pluginKey, | |
| information: versionable.version | |
| ) | |
| } else { | |
| pluginInfoItem = PluginInfoItem( | |
| displayName: pluginKey, | |
| information: DevMenuStringConstants.versionNotAvailable | |
| ) | |
| } | |
| return pluginInfoItem | |
| } | |
| guard let versionable = versionable else { | |
| return PluginInfoItem( | |
| displayName: pluginKey, | |
| information: DevMenuStringConstants.versionNotAvailable | |
| ) | |
| } | |
| return PluginInfoItem(displayName: pluginKey, information: versionable.version) | |
| } |
There was a problem hiding this comment.
Hmm. In this case, I'm less instinctively inclined to apply a guard because it's not actually an early exit. But thinking about it more, I like how it makes the "I need this info to do the happy path" intent easier to follow, so yeah! Guard! :)
There was a problem hiding this comment.
Thinking about it even more though, it's even cleaner to just worry about the version:
let version = versionable?.version ?? DevMenuStringConstants.versionNotAvailable
return PluginInfoItem(displayName: pluginKey, information: version)| let bundle = Bundle(for: type(of: self)) | ||
| let version = bundle.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String | ||
| return version ?? "Not Available" |
There was a problem hiding this comment.
Can we give default behavior for this protocol, instead of repeating the same code?
There was a problem hiding this comment.
When I originally thought about that I was stymied by type(of:) expecting a class, and AmplifyVersionable not being constrained to classes. But now that I'm thinking about it, I could write a conditional extension. Derp. Will fix.
Issue #, if available:
Refs: #850
Description of changes:
Several changes to correctly populate the dev menu:
AmplifyVersionableconformances to all AWS pluginsAmplifyVersionablepublic so it could be consumed by pluginsEnvironmentInfooptional so missing fields wouldn't cause the entire thing to not displaySee screenshot below for populated menu. Note that I had to manually update the
amplify/.config/local-env-info.jsonfile, since the CLI is not yet populating those items (at least on my system).By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.