Skip to content

Conversation

@devversion
Copy link
Member

Follow-up commit for 4879dc4 since angular/dgeni-packages#270 has been merged and published.


Disables concatenation of multiple leading comments for a TypeScript node. Since all shipped source files have a license banner at top, the license banner comment would be incorrectly considered as "comment" for the first TypeScript node of a given file. Since there are various files in the Material project where the first node of a source file is exported and should only use the first leading comment, we need to disable comment concatenation.

See for example: https://github.com/angular/material2/blob/master/src/cdk/coercion/boolean-property.ts

@devversion devversion added pr: merge safe target: patch This PR is targeted for the next patch release labels Aug 29, 2018
@devversion devversion requested a review from jelbourn as a code owner August 29, 2018 11:49
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 29, 2018
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@devversion devversion force-pushed the build/dgeni-concat-leading-comments branch from 25d65f4 to 92a3713 Compare August 29, 2018 20:25
@devversion devversion added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Aug 29, 2018
@devversion
Copy link
Member Author

@jelbourn Rebased. I'm adding merge ready. Let me know if there is anything that we should wait on.

@jelbourn jelbourn removed the action: merge The PR is ready for merge by the caretaker label Aug 29, 2018
@jelbourn
Copy link
Member

@devversion travis is red across the board

@devversion
Copy link
Member Author

devversion commented Aug 29, 2018

Seems to be because of postinstall trying to check the typings of @angular-devkit/core (this now appears because I've updated the package-lock and locally I got this as an explicit error)

See: https://unpkg.com/@angular-devkit/[email protected]/src/virtual-fs/host/test.d.ts

@devversion devversion force-pushed the build/dgeni-concat-leading-comments branch from 5a95f12 to a31f782 Compare August 29, 2018 22:07
@devversion
Copy link
Member Author

devversion commented Aug 29, 2018

@jelbourn Fixed. For this PR, I'm just updating the package-lock to reflect the dgeni-packages update.

As mentioned in the comment above, previously I updated all packages and something broke. This is something that should be fixed separately.

Follow-up commit for 4879dc4.

---

Disables concatenation of multiple leading comments for a TypeScript node. Since all shipped source files have a license banner at top, the license banner comment would be incorrectly considered as "comment" for the first TypeScript node of a given file. Since there are various files in the Material project where the first node of a source file is exported and should only use the first leading comment, we need to disable comment concatenation.

See for example: https://github.com/angular/material2/blob/master/src/cdk/coercion/boolean-property.ts
@devversion devversion force-pushed the build/dgeni-concat-leading-comments branch from a31f782 to d3cc38f Compare August 29, 2018 22:12
@devversion devversion added the action: merge The PR is ready for merge by the caretaker label Aug 29, 2018
@jelbourn jelbourn merged commit 53b4af6 into angular:master Aug 29, 2018
@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 9, 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.

3 participants