erisu commented on code in PR #1406: URL: https://github.com/apache/cordova-ios/pull/1406#discussion_r1513991589
########## lib/ConfigParseriOS.js: ########## Review Comment: Rename to `PlatformConfigParser.js` ########## lib/ConfigParseriOS.js: ########## @@ -0,0 +1,37 @@ + +/** + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +*/ + +'use strict'; + +const ConfigParser = require('cordova-common').ConfigParser; + +class ConfigParseriOS extends ConfigParser { Review Comment: ```suggestion class PlatformConfigParser extends ConfigParser { ``` IMO I think it would be better to make the class name generic. For example, if Android implement its own extended config parser class then it would follow identical naming conventions. Almost as if we created and followed a platform template and would be easy to follow if we create a new platform in the future. ########## lib/prepare.js: ########## @@ -35,6 +36,7 @@ const FileUpdater = require('cordova-common').FileUpdater; const projectFile = require('./projectFile'); const Podfile = require('./Podfile').Podfile; const check_reqs = require('./check_reqs'); +const ConfigParseriOS = require('./ConfigParseriOS'); Review Comment: If class name changes, update here as well. ########## lib/prepare.js: ########## @@ -43,9 +45,16 @@ const CDV_ANY_SIZE_CLASS = 'any'; module.exports.prepare = function (cordovaProject, options) { const platformJson = PlatformJson.load(this.locations.root, 'ios'); const munger = new PlatformMunger('ios', this.locations.root, platformJson, new PluginInfoProvider()); - this._config = updateConfigFile(cordovaProject.projectConfig, munger, this.locations); + const parser = new ConfigParseriOS(cordovaProject.projectConfig.path); Review Comment: If class name changes, update here as well. -- 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-unsubscr...@cordova.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
