feat(mobile-app): Remove unstable-mobile-app feature gating #2601
Conversation
rbro112
changed the title
|
I will wait to review this until our sync meeting next week, so I can provide more informed feedback. Please let me know if you need me to review sooner |
rbro112
force-pushed
the
ryan/remove_unstable-mobile-app_feature_gating
branch
from
d5dae53 to
9ecb724
Compare
| [dependencies] | ||
| anylog = "0.6.3" | ||
| anyhow = { version = "1.0.69", features = ["backtrace"] } | ||
| apple-catalog-parsing = { path = "apple-catalog-parsing", optional = true } |
There was a problem hiding this comment.
@szokeasaurusrex, I had to remove the optional flag to get this to compile properly. Admittedly I'm a bit unsure of the effect that removing this optionality would have as it pertains to the release (and how you had the dependency seemingly conditionally included with the unstable-mobile-app feature), so if you don't mind a check/fix here, that would be great.
There was a problem hiding this comment.
I think this is fine. The crate should compile on all platforms; on non-macOS platforms it is just an empty crate.
I can try to see though if I can add it as a macOS-specific dependency
.github/workflows/lint.yml
Outdated
| display-os: macOS | ||
| - os: windows-2022 | ||
| display-os: Windows | ||
| - feature-args: '' |
There was a problem hiding this comment.
These were originally added in #2582, but I left these in as the empty args could be useful for having around if future flags are added so ${{ matrix.feature-suffix }} won't need to be added below again.
Will leave to you @szokeasaurusrex to remove if you'd prefer.
There was a problem hiding this comment.
Let's remove – otherwise, if we never end up using this again, future maintainers could be confused about why this even exists.
We can always add this back if needed
.github/workflows/test.yml
Outdated
| display-os: macOS | ||
| - os: windows-2022 | ||
| display-os: Windows | ||
| - feature-args: '' |
There was a problem hiding this comment.
Similar to lint.yml, left these in as I figured it's easier to leave around for future feature flags
szokeasaurusrex
left a comment
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Looks good after my minor changes, thanks!
…e-mobile-app_feature_gating
szokeasaurusrex
deleted the
ryan/remove_unstable-mobile-app_feature_gating
branch
We're ready to start end-to-end testing this, so removing the
unstable-mobile-appflag that was added in #2582 to re-enable themobile-appcommand without the explicit flag.