Skip to content

Conversation

@rkirov
Copy link
Contributor

@rkirov rkirov commented Feb 15, 2019

…cess.

This is a strictness check that we are going to enforce in g3 internal
typescript to aid with optimization. Generally, it is also syntactically
a good idea to have visual indication when a type is accessed through an
index signature vs not.

…cess.

This is a strictness check that we are going to enforce in g3 internal
typescript, to aid with optimization. Generally it is also syntactically
a good idea to have visual indication when a type is accessed through an
index signature vs not.
@rkirov rkirov requested a review from jelbourn February 15, 2019 23:51
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 15, 2019
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto crisbeto added pr: lgtm action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Feb 16, 2019
@rkirov rkirov merged commit 5216d38 into angular:master Feb 19, 2019
@rkirov
Copy link
Contributor Author

rkirov commented Feb 19, 2019

Jeremy tells me I shouldn't have clicked on the big green button. Apologies to the caretaker, feel free to rollback as needed.

@devversion
Copy link
Member

devversion commented Feb 20, 2019

I'm thinking if it would be possible to create a lint rule for this that enforces this type of property access. At first glance it looks like we could easily end up introducing new incorrect property accesses.

jelbourn pushed a commit that referenced this pull request Feb 20, 2019
…cess. (#15206)

This is a strictness check that we are going to enforce in g3 internal
typescript, to aid with optimization. Generally it is also syntactically
a good idea to have visual indication when a type is accessed through an
index signature vs not.
rkirov added a commit to rkirov/material2 that referenced this pull request Mar 19, 2019
This is a follow up to angular#15206,
with these two changes all of material in conforming.

A build-time check to enforce this is part of tsetse -
https://tsetse.info/property-renaming-safe but I don't know how to turn
it on for this repositories bazel builds.
@rkirov
Copy link
Contributor Author

rkirov commented Mar 19, 2019

Better than a lint rule, we have a bazel tsetse check - https://tsetse.info/property-renaming-safe, however I don't know how to turn it on for your repo.

andrewseguin pushed a commit that referenced this pull request Mar 20, 2019
…#15523)

This is a follow up to #15206,
with these two changes all of material in conforming.

A build-time check to enforce this is part of tsetse -
https://tsetse.info/property-renaming-safe but I don't know how to turn
it on for this repositories bazel builds.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Mar 30, 2019
Adds a lint rule to help catch cases like angular#15206 until we can get something going through tsetse. Note this is only scoped to `ngOnChanges` and accesses of the `SimpleChanges` objects since that's where we've been having issues.
mmalerba pushed a commit that referenced this pull request Apr 1, 2019
…15659)

Adds a lint rule to help catch cases like #15206 until we can get something going through tsetse. Note this is only scoped to `ngOnChanges` and accesses of the `SimpleChanges` objects since that's where we've been having issues.
@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 Sep 10, 2019
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 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