Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "ci: disable lint check in the update test" #18944

Merged
merged 1 commit into from Oct 15, 2020

Conversation

dgp1130
Copy link
Collaborator

@dgp1130 dgp1130 commented Oct 1, 2020

Re-enabling the lint check to merge once 11.0.0-next.4 is released.

I'll leave this PR in the blocked state until the version is released and angular/angular#39070 is merged. Once those have both happened, it should be safe to merge this as well.

@dgp1130 dgp1130 added PR state: blocked target: major This PR is targeted for the next major release labels Oct 1, 2020
@dgp1130 dgp1130 requested a review from clydin October 1, 2020 00:09
@dgp1130
Copy link
Collaborator Author

dgp1130 commented Oct 2, 2020

The relevant blocking PR was merged, however it was reverted in angular/angular#39082 because it is not compatible with TS 3.9 which is still used in google3. I'm looking for a solution that will be compatible with both versions and we can re-enable this test once that lands.

@dgp1130
Copy link
Collaborator Author

dgp1130 commented Oct 2, 2020

I have a fix in angular/angular#39102, where using ts.createIdentifier() is able to make a single quoted string literal in both TS 3.9 and TS 4.0. This should be compatible with google3.

@dgp1130
Copy link
Collaborator Author

dgp1130 commented Oct 9, 2020

The fix was merged and doesn't appear to have been rolled back yet, so I think it's safe to merge this PR and re-enable the lint check.

@dgp1130
Copy link
Collaborator Author

dgp1130 commented Oct 9, 2020

Actually, CI is still failing for this PR itself, because the fix on angular/angular has not been released and included in the CLI. I think we'll also need to wait for that, so I'll need to follow up next week on this.

This reverts commit 4271129.

Re-enables lint check now that v11.0.0-next.4 has released.
@dgp1130
Copy link
Collaborator Author

dgp1130 commented Oct 14, 2020

@alan-agius4 pointed out that e2e tests load FW from NPM, so we only need FW to release the migration fix, not the CLI.

This should be unblocked now.

@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Oct 14, 2020
@alan-agius4 alan-agius4 merged commit 2536409 into angular:master Oct 15, 2020
@dgp1130 dgp1130 deleted the lint-enable branch October 15, 2020 18:02
@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 Nov 15, 2020
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 target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants