[TIMOB-24027] Provide migration help for new properties (2_0_X) #92
Conversation
| @@ -71,6 +71,7 @@ function HyperloopiOSBuilder(logger, config, cli, appc, hyperloopConfig, builder | |||
| this.nativeModules = {}; | |||
| this.buildSettings = {}; | |||
| this.headers = null; | |||
| this.needMigration = {}; | |||
There was a problem hiding this comment.
I prefer needsMigration
There was a problem hiding this comment.
needsMigration would infer that this variable contains a single value, but it actually contains all methods that need migration so i left it at needMigration.
| that.logger.error('!!! CODE MIGRATION REQUIRED !!!'); | ||
| that.logger.error(); | ||
| that.logger.error('Due to changes introduced in iOS 10 and Hyperloop 2.0.0 some method calls need'); | ||
| that.logger.error('to be changed to property access. It seems like you used some of the affected'); |
There was a problem hiding this comment.
to be changed to property access. => to be changed regarding the access of properties.
| that.logger.error('to be changed to property access. It seems like you used some of the affected'); | ||
| that.logger.error('methods.'); | ||
| that.logger.error(); | ||
| that.logger.error('We tried to fix most of these automatically during our Copy Resources build step.'); |
There was a problem hiding this comment.
during our Copy Resources build step. => during compile time. (They don't know about our build phases :-)
| that.logger.error('methods.'); | ||
| that.logger.error(); | ||
| that.logger.error('We tried to fix most of these automatically during our Copy Resources build step.'); | ||
| that.logger.error('However, we did not touch your original source files. Please see the list bellow'); |
There was a problem hiding this comment.
bellow => below. bellow means brüllen in german btw :-)
| that.logger.error('However, we did not touch your original source files. Please see the list bellow'); | ||
| that.logger.error('to help you migrate your code.'); | ||
| that.logger.error(); | ||
| that.logger.error('NOTE: Some line numbers and filenames shown here are from your compiled alloy files'); |
There was a problem hiding this comment.
filenames => file names (https://www.dict.cc/deutsch-englisch/Dateiname.html)
There was a problem hiding this comment.
alloy files => Alloy source code (capitalized and different wording)
There was a problem hiding this comment.
I always wondered what's correct, filename or file name. Guess you can use both but filename is slightly more common nowdays: http://english.stackexchange.com/questions/5366/which-is-correct-filename-file-name-or-filename
| that.logger.error('to help you migrate your code.'); | ||
| that.logger.error(); | ||
| that.logger.error('NOTE: Some line numbers and filenames shown here are from your compiled alloy files'); | ||
| that.logger.error('and may differ from your original source file.'); |
There was a problem hiding this comment.
source file => source code
| Object.keys(this.needMigration).forEach(function (pathAndFilename) { | ||
| var tokens = that.needMigration[pathAndFilename]; | ||
| that.logger.error(); | ||
| var shortPathAndFilename = pathAndFilename.replace(that.resourcesDir, 'Resources'); |
There was a problem hiding this comment.
Arrange the variables together for better code-style and use error('') to signalize the empty line more clear.
| @@ -0,0 +1,290 @@ | |||
| { | |||
| return getMethodTableForMigration.cachedTable; | ||
| } | ||
|
|
||
| var migrationPathAndFilename = path.resolve(__dirname, '../../data/migration-20161014143619.json'); |
There was a problem hiding this comment.
Don't hard-code the file name inside this method. Rather place it at the top or introduce a config-file for this kind of stuff.
|
Left some minor comments. Please also do the master-PR, thx. |
janvennemann
changed the title
|
CR + FT passed, PR approved! |
JIRA: https://jira.appcelerator.org/browse/TIMOB-24027
Show migration instructions for the changes introduced in #82 and try to fix most of them automatically during build phase so the existing code won't crash.