INTL-1608 : Activate _over_react_codemod executables separately #246
Conversation
akshaybhardwaj-wk
requested review from
alanknight-wk and
chetansinghchoudhary-wk
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
lib/src/executables/sort_intl.dart
Outdated
| // Example: | ||
| // final intlPropMigrator = IntlMigrator(messages.className, messages); | ||
|
|
||
| Future<int> runMigrators(List<String> packageDartPaths, |
There was a problem hiding this comment.
Yes, and I'm saying we shouldn't run migrators at all.
dustinlessard-wf
changed the title
lib/src/executables/sort_intl.dart
Outdated
| final packageName = packageNameLookup[packageRoot] ?? 'fix_me_bad_name'; | ||
| _log.info('Starting migration for $packageName'); | ||
| final IntlMessages messages = IntlMessages(packageName); | ||
| messages.write(); |
There was a problem hiding this comment.
I think it would be a good idea to put force: true on the write call, so it will always write.
lib/src/executables/sort_intl.dart
Outdated
| for (String package in packageRoots) { | ||
| final packageRoot = p.basename(package); | ||
| final packageName = packageNameLookup[packageRoot] ?? 'fix_me_bad_name'; | ||
| _log.info('Starting migration for $packageName'); |
There was a problem hiding this comment.
The logger info doesn't actually print anything. I would just turn this into a print statement.
And change the text to something like Sorting and formatting INTL files for $packageName.
lib/src/executables/sort_intl.dart
Outdated
| final packageRoot = p.basename(package); | ||
| final packageName = packageNameLookup[packageRoot] ?? 'fix_me_bad_name'; | ||
| _log.info('Starting migration for $packageName'); | ||
| final IntlMessages messages = IntlMessages(packageName); |
There was a problem hiding this comment.
And maybe add a print statement that says something like
Wrote ${messages.outputFile.path} so it's clear which file(s) were changed
lib/src/executables/sort_intl.dart
Outdated
| } | ||
|
|
||
| void printUsage() { | ||
| stderr.writeln('Activating Excutables and Sort INTL file.'); |
There was a problem hiding this comment.
Should be more like 'Sort and format INTL files'
lib/src/executables/sort_intl.dart
Outdated
| return; | ||
| } | ||
|
|
||
| // If we have specified paths on the command line, limit our processing to |
There was a problem hiding this comment.
There are two cases we have to consider for this. The first is when we have a normal repo with a single package that has a file lib/src/intl/packagename_intl.dart in it. For example, wdesk. This executable works for that. But the reason it works is that this code finds every Dart file underneath /lib and finds a single package root which is the root of the repo. But in that case, finding the package root is not really necessary, because it's the known root of the repo.
The more complicated case is when a repo has multiple subpackages, for example ml_ui_client. In that case finding the package roots would be useful, but this doesn't do it properly because it only looks for files underneath /lib. There is no top-level lib directory in ml_ui_client, so it doesn't find anything and throws an exception. Could not find package root for file /Users/alanknight/dart/ml_ui_client/lib
lib/src/executables/sort_intl.dart
Outdated
| } | ||
|
|
||
| // If we have specified paths on the command line, limit our processing to | ||
| // those, and make sure they're absolute. |
There was a problem hiding this comment.
So there are two directions this could go.
- Don't find all the Dart files. Find all the _intl.dart files under the root directory, but exclude test. Then I think this code would more or less work.
- Don't do any of this. Do what I said the last time, which you marked as resolved without doing it or explaining. The only difficulty is that it requires the package name. But the only thing that's used for is to name the intl file. So you could derive the package name from the pubspec as before, but knowing that it's in the root directory and there's only one. Or you could derive it from the name of the intl.dart file, whose position is known.
There was a problem hiding this comment.
@alanknight-wk I have updated the code
- To derive the package name from
pubspec.yamlcreate the following method - Also tested in ml_ui_client if we run this executable at root level where no
pubspec.yamlthis shows an error
could not determine the package name but underml_ui_clientdirectory it works . - Also tested in DPC where we have
pubspec.yamlon root it not shows error but sorting not working because intl.dart files are undersubpackage/package/lib/srceg:subpackage/datatables/lib/src
|
@Workiva/release-management-p |
|
QA +1 |
Problem
AS per Ticket INTL-1322 we need to run the sorting command with
ddev formatSolution
lib/src/executables/sort_intl.dartbin/sort_intl.dartHow To QA
A. Local system : To check this in the Local system as debug mode do the following steps
over_react_codemodin local system as debug mode run the commanddart pub global activate --source path over_react_codemoddart pub global activate --source path . -x sort_intl
this will show message on terminal
Also added
intl_sortingas an executable inpubspec.yamlhereRun
codemodwith executables only in any repo run this commanddart pub global run over_react_codemod: sort_intlB. As a global package (This will work when this logic done as a Package in
pub.workiva)over_react_codemodrun this commanddart pub global activate over_react_codemoddart pub global activate over_react_codemod -x sort_intl