Skip to content

Conversation

devversion
Copy link
Member

  • Removes stale scope checking (from @angular2-material/$COMPONENT times)
  • Adds support for error grouping (useful when having a file full of invalid line endings)
  • Moves chain functions into external functions to easily


understand procession.

Here you can see the error grouping - Notice that we don't show any special message for the line endings (because to keep the script small and easy to maintain)

* Removes stale scope checking (from `@angular2-material/$COMPONENT` times)
* Adds support for error grouping (useful when having a file full of invalid line endings)
* Moves chains into exta functions to understand procession easily.
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 4, 2016
return initialError.fileName === error.fileName &&
initialError.statement === error.statement &&
previousLine === (error.lineNumber - 1);
}
Copy link
Member Author

@devversion devversion Nov 4, 2016

Choose a reason for hiding this comment

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

I know this is not very pretty, but we don't have any Lodash helpers here, and this seems to be one elegant approach.

@jelbourn
Copy link
Member

@hansl can you take a look?

@devversion
Copy link
Member Author

@hansl Do you have time to take a quick look at this? :)

@hansl
Copy link
Contributor

hansl commented Nov 28, 2016

Hi @devversion,

Sorry, this fell off a bit. Looking at it now.

@hansl
Copy link
Contributor

hansl commented Nov 29, 2016

LGTM for now.

@devversion What I'd like to see though is moving away from a forbidden-identifier file and using custom tslint rules instead. It would be better and less hacky (since reporting is done by tslint, for example). See documentation here: https://www.npmjs.com/package/tslint#custom-rules

What do you think?

@devversion
Copy link
Member Author

@hansl - Great idea. I can do a follow-up PR to this one with the custom TS-Lint rules-

@hansl hansl added the action: merge The PR is ready for merge by the caretaker label Nov 29, 2016
@hansl
Copy link
Contributor

hansl commented Nov 29, 2016

Yes, a follow up PR. This one is okay (for now).

@tinayuangao tinayuangao merged commit f1b9b2c into angular:master Nov 29, 2016
@devversion devversion deleted the chore/forbidden-identifiers-lf branch November 29, 2016 19:19
@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 6, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants