Remove some StringMapWrapper facades #12022

alexeagle · 2016-10-01T00:41:40Z

Automated using alexeagle/tslint@12012b0
with a few manual fixes after.

vicb · 2016-10-01T01:24:23Z

modules/@angular/benchpress/src/metric/multi_metric.ts

-  maps.forEach(
-      map => { StringMapWrapper.forEach(map, (value, prop) => { result[prop] = value; }); });
+  maps.forEach(map => {
+    Object.keys(map).forEach(prop => {


remove "{", convert to one liner

The advantage of the curly braces is they either contain a return statement or are void. typescript-eng has discussed the style here and I think we've decided that arrow function with a bare expression on the RHS means you use the return value from it. Otherwise it is unclear if you are using an expression to mean void, or are using its return value.

Ok for braces but still should be 1 liner, wdyt

It's up to clang-format to decide how many lines... do you just mean formatting, or do you mean to change syntax?

Change the syntax, do not introduce the const

vicb · 2016-10-01T01:25:04Z

modules/@angular/benchpress/src/reporter/util.ts

@@ -17,7 +17,10 @@ export function formatNum(n: number) {

 export function sortedProps(obj: {[key: string]: any}) {
  var props: string[] = [];
-  StringMapWrapper.forEach(obj, (value, prop) => props.push(prop));
+  Object.keys(obj).forEach(prop => {


unused value, one liner

I fixed these cases where I created a value variable that was not used, PTAL

Push(...Object.keys)

vicb · 2016-10-01T01:25:16Z

modules/@angular/benchpress/src/sample_description.ts

@@ -42,7 +41,10 @@ export class SampleDescription {
      public metrics: {[key: string]: any}) {
    this.description = {};
    descriptions.forEach(description => {
-      StringMapWrapper.forEach(description, (value, prop) => this.description[prop] = value);
+      Object.keys(description).forEach(prop => {


vicb · 2016-10-01T01:25:56Z

modules/@angular/benchpress/test/metric/perflog_metric_spec.ts

@@ -68,7 +67,10 @@ export function main() {

    function sortedKeys(stringMap: {[key: string]: any}) {
      var res: string[] = [];
-      StringMapWrapper.forEach(stringMap, (_, key) => { res.push(key); });
+      Object.keys(stringMap).forEach(key => {


Res.push(...Object.keys)

vicb

Most 2 liners should be converted to 1 line IMO
Did not repeat the same comments towards the end

vicb · 2016-10-01T05:19:50Z

modules/@angular/benchpress/src/reporter/util.ts

@@ -17,7 +17,10 @@ export function formatNum(n: number) {

 export function sortedProps(obj: {[key: string]: any}) {
  var props: string[] = [];
-  StringMapWrapper.forEach(obj, (value, prop) => props.push(prop));
+  Object.keys(obj).forEach(prop => {


Push(...Object.keys)

vicb · 2016-10-01T05:20:14Z

modules/@angular/benchpress/src/sample_description.ts

@@ -42,7 +41,10 @@ export class SampleDescription {
      public metrics: {[key: string]: any}) {
    this.description = {};
    descriptions.forEach(description => {
-      StringMapWrapper.forEach(description, (value, prop) => this.description[prop] = value);
+      Object.keys(description).forEach(prop => {


vicb · 2016-10-01T05:20:34Z

modules/@angular/benchpress/test/metric/perflog_metric_spec.ts

@@ -68,7 +67,10 @@ export function main() {

    function sortedKeys(stringMap: {[key: string]: any}) {
      var res: string[] = [];
-      StringMapWrapper.forEach(stringMap, (_, key) => { res.push(key); });
+      Object.keys(stringMap).forEach(key => {


vicb · 2016-10-01T05:21:05Z

modules/@angular/benchpress/test/metric/perflog_metric_spec.ts

@@ -68,7 +67,10 @@ export function main() {

    function sortedKeys(stringMap: {[key: string]: any}) {
      var res: string[] = [];
-      StringMapWrapper.forEach(stringMap, (_, key) => { res.push(key); });
+      Object.keys(stringMap).forEach(key => {


Res.push(...Object.keys)

vicb · 2016-10-01T05:24:10Z

modules/@angular/compiler/src/animation/animation_parser.ts

-            }
-          });
+      Object.keys(entry).forEach(prop => {
+        const value: any = (entry as{[key: string]: string | number})[prop];


vicb · 2016-10-01T05:24:26Z

modules/@angular/compiler/src/animation/animation_parser.ts

-          }
-        });
+    Object.keys(styles).forEach(prop => {
+      const value = styles[prop];


alexeagle · 2016-10-03T17:52:31Z

I did a bunch more manual cleanup, mostly removing the introduced const value if it was only used once.

vicb · 2016-10-03T22:45:15Z

@alexeagle could you please rebase

This change mostly automated by alexeagle/tslint@12012b0 with some manual fixes.

alexeagle · 2016-10-03T23:02:56Z

rebased

On Mon, Oct 3, 2016 at 3:45 PM Victor Berchet notifications@github.com
wrote:

@alexeagle https://github.com/alexeagle could you please rebase

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#12022 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC5I1T7dOEv854HPO_TozfI5-Do6ET8ks5qwYV9gaJpZM4KLn6b
.

) This change mostly automated by alexeagle/tslint@12012b0 with some manual fixes.

This change mostly automated by alexeagle/tslint@12012b0 with some manual fixes.

angular-automatic-lock-bot · 2019-09-09T21:21:13Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

alexeagle added the action: review The PR is still awaiting reviews from at least one requested reviewer label Oct 1, 2016

alexeagle assigned vicb Oct 1, 2016

googlebot added the cla: yes label Oct 1, 2016

alexeagle force-pushed the rm_isblank branch from 69dc611 to 61b35c3 Compare October 1, 2016 00:44

vicb suggested changes Oct 1, 2016

View reviewed changes

alexeagle force-pushed the rm_isblank branch from 61b35c3 to 11e5ecf Compare October 1, 2016 03:51

vicb reviewed Oct 1, 2016

View reviewed changes

alexeagle force-pushed the rm_isblank branch 2 times, most recently from 974d179 to 5bc2ad6 Compare October 3, 2016 17:51

alexeagle force-pushed the rm_isblank branch from 5bc2ad6 to 350ecd4 Compare October 3, 2016 18:53

vicb added pr_state: LGTM action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 3, 2016

vicb approved these changes Oct 3, 2016

View reviewed changes

chuckjaz added the action: rebase label Oct 3, 2016

refactor(facade): Remove most of StringMapWrapper facade.

2abfc35

This change mostly automated by alexeagle/tslint@12012b0 with some manual fixes.

alexeagle force-pushed the rm_isblank branch from 350ecd4 to 2abfc35 Compare October 3, 2016 23:02

chuckjaz removed the action: rebase label Oct 3, 2016

chuckjaz merged commit b64b5ec into angular:master Oct 3, 2016

chuckjaz pushed a commit to chuckjaz/angular that referenced this pull request Oct 5, 2016

refactor(facade): Remove most of StringMapWrapper facade. (angular#12022

f5b2e2f

) This change mostly automated by alexeagle/tslint@12012b0 with some manual fixes.

chuckjaz pushed a commit that referenced this pull request Oct 5, 2016

refactor(facade): Remove most of StringMapWrapper facade. (#12022)

1bd8ba8

This change mostly automated by alexeagle/tslint@12012b0 with some manual fixes.

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove some StringMapWrapper facades #12022

Remove some StringMapWrapper facades #12022

alexeagle commented Oct 1, 2016

vicb Oct 1, 2016

alexeagle Oct 1, 2016

vicb Oct 1, 2016

alexeagle Oct 3, 2016

vicb Oct 3, 2016

vicb Oct 1, 2016

alexeagle Oct 1, 2016

vicb Oct 1, 2016

vicb Oct 1, 2016

vicb Oct 1, 2016

vicb Oct 1, 2016

vicb Oct 1, 2016

vicb Oct 1, 2016

vicb left a comment

vicb Oct 1, 2016

vicb Oct 1, 2016

vicb Oct 1, 2016

vicb Oct 1, 2016

vicb Oct 1, 2016

vicb Oct 1, 2016

alexeagle commented Oct 3, 2016

vicb commented Oct 3, 2016

alexeagle commented Oct 3, 2016

angular-automatic-lock-bot bot commented Sep 9, 2019

Remove some StringMapWrapper facades #12022

Remove some StringMapWrapper facades #12022

Conversation

alexeagle commented Oct 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vicb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexeagle commented Oct 3, 2016

vicb commented Oct 3, 2016

alexeagle commented Oct 3, 2016

angular-automatic-lock-bot bot commented Sep 9, 2019