You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by "dpogue (via GitHub)" <gi...@apache.org> on 2024/08/15 10:20:23 UTC

[PR] refactor!: Add CDVSettingsDictionary class [cordova-ios]

dpogue opened a new pull request, #1458:
URL: https://github.com/apache/cordova-ios/pull/1458

   ### To Do
   - [ ] Write unit tests
   
   ### Platforms affected
   iOS
   
   
   ### Motivation and Context
   <!-- Why is this change required? What problem does it solve? -->
   <!-- If it fixes an open issue, please link to the issue here. -->
   This will replace the extension category on NSDictionary, which has always felt a little bit hacky and has caused issues in weird circumstances due to requiring special build flags to work properly.
   
   
   ### Description
   <!-- Describe your changes in detail -->
   Add a new `CDVSettingsDictionary` class that is used for accessing preferences.
   
   Yes, subclassing `NSDictionary` is generally considered a bad idea because the internal implementation details are weird and cause problems. However, in this case we're essentially wrapping a real `NSMutableDictionary` and proxying everything to it, so this _should_ work the way we'd expect it to.
   
   
   ### Testing
   <!-- Please describe in detail how you tested your changes. -->
   TODO
   
   
   ### Checklist
   
   - [ ] I've run the tests to see all new and existing tests pass
   - [ ] I added automated test coverage as appropriate for this change
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org

Re: [PR] refactor!: Add CDVSettingsDictionary class [cordova-ios]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1458:
URL: https://github.com/apache/cordova-ios/pull/1458#issuecomment-2291044707

   ## [Codecov](https://app.codecov.io/gh/apache/cordova-ios/pull/1458?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   All modified and coverable lines are covered by tests :white_check_mark:
   > Project coverage is 78.35%. Comparing base [(`5089892`)](https://app.codecov.io/gh/apache/cordova-ios/commit/508989285a6bc39a0e67c1333fd310137e91c2a9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`e4f044d`)](https://app.codecov.io/gh/apache/cordova-ios/commit/e4f044dda8686d2a80f28921e39c33cfb0fd7f35?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 1 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #1458   +/-   ##
   =======================================
     Coverage   78.35%   78.35%           
   =======================================
     Files          16       16           
     Lines        1825     1825           
   =======================================
     Hits         1430     1430           
     Misses        395      395           
   ```
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cordova-ios/pull/1458?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org

Re: [PR] refactor!: Add CDVSettingsDictionary class [cordova-ios]

Posted by "dpogue (via GitHub)" <gi...@apache.org>.
dpogue merged PR #1458:
URL: https://github.com/apache/cordova-ios/pull/1458


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org