Skip to content

cleanup: migrate migration tool to schematics #10736

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

Merged
merged 11 commits into from
Apr 11, 2018
Merged

Conversation

mmalerba
Copy link
Contributor

@mmalerba mmalerba commented Apr 6, 2018

Note: everything in the update/ directory other than update.ts is lifted out of https://github.com/angular/material-update-tool

@mmalerba mmalerba requested a review from hansl April 6, 2018 17:02
@mmalerba mmalerba requested review from amcdnl and jelbourn as code owners April 6, 2018 17:02
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 6, 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.

Let me know if you run into issues w/ the bazel stuff

@@ -9,6 +9,10 @@
"schema": "./shell/schema.json",
"aliases": ["material-shell"]
},
"ng-update": {
"description": "Attempts to make fixes to the application to make upgrading from Angular Material 5 to 6 easier",
Copy link
Member

Choose a reason for hiding this comment

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

How about

Updates API usage for the most recent major version of
Angular CDK and Angular Material

import {TslintFixTask} from '@angular-devkit/schematics/tasks';
import * as path from 'path';

export default function(): Rule {
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment like Entry point for `ng update` from Angular CLI?

@mmalerba mmalerba requested a review from devversion as a code owner April 6, 2018 21:05
amcdnl
amcdnl previously requested changes Apr 9, 2018
const moduleSpecifier = declaration.moduleSpecifier.getText();
return moduleSpecifier.indexOf(materialModuleSpecifier) !== -1||
moduleSpecifier.indexOf(cdkModuleSpecifier) !== -1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should have newline at end.

data => data.replace === symbol.name || data.replace === identifier.text);

if (!classData) {
console.error(`Could not find updated name for identifier "${identifier.getText()}" in ` +
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we console.error or throw error here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

console.error, we still want to continue trying to detect and fix other issues

} else {
return resolveBinSync('tslint', 'tslint');
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should end w/ new line.

@mmalerba mmalerba changed the title WIP: migrate migration tool to schematics cleanup: migrate migration tool to schematics Apr 10, 2018
@mmalerba
Copy link
Contributor Author

@jelbourn @hansl This should be ready now

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

@jelbourn jelbourn 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 and removed pr: needs review labels Apr 10, 2018
@tinayuangao
Copy link
Contributor

@mmalerba Please rebase, thanks

@mmalerba
Copy link
Contributor Author

@tinayuangao done

@tinayuangao tinayuangao merged commit 3cc21cd into angular:master Apr 11, 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 8, 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.

6 participants