Skip to content

Conversation

@crisbeto
Copy link
Member

Reworks the internal setup of the google-map component so that we don't have to declare observables for each input.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Nov 18, 2020
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 18, 2020
Reworks the internal setup of the `google-map` component so that we don't have to
declare observables for each input.
@crisbeto crisbeto force-pushed the google-map-refactor branch from 14ca61f to 6e944ee Compare November 18, 2020 20:17
@crisbeto crisbeto requested a review from a team November 18, 2020 20:17
this._setSize();
}

const googleMap = this.googleMap;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All things being equal, I think we may as well just continue to reference this.googleMap instead of reassigning it to a local variable.

Copy link
Member Author

@crisbeto crisbeto Nov 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit of a micro-optimization, but I wrote it this way because it'll minify better with most common minifiers that don't do property renaming.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment saying that was the reason for doing this?

@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Nov 19, 2020
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

this._setSize();
}

const googleMap = this.googleMap;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment saying that was the reason for doing this?

@annieyw annieyw merged commit 28c36f8 into angular:master Feb 9, 2021
annieyw pushed a commit that referenced this pull request Feb 9, 2021
Reworks the internal setup of the `google-map` component so that we don't have to
declare observables for each input.

(cherry picked from commit 28c36f8)
annieyw pushed a commit that referenced this pull request Feb 9, 2021
Reworks the internal setup of the `google-map` component so that we don't have to
declare observables for each input.

(cherry picked from commit 28c36f8)
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants