Skip to content

AutoRest: update autorest.java cmd #6735

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.

Sign up for GitHub

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
xseeseesee merged 3 commits into from
Dec 11, 2019
Merged

AutoRest: update autorest.java cmd #6735

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6be26b5
AutoRest: update autorest.java cmd
xseeseesee Dec 10, 2019
7006b53
AutoRest: update by review comments
xseeseesee Dec 10, 2019
4e90a11
AutoRest: restrict version number validation
xseeseesee Dec 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions eng/mgmt/gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ gulp.task('default', function() {
console.log("\tThe version of AutoRest. E.g. 2.0.9, or the location of AutoRest repo, e.g. E:\\repo\\autorest");

console.log("--autorest-java");
console.log("\tPath to an autorest.java generator to pass as a --use argument to AutoRest.");
console.log("\tOption#1: Path to an autorest.java generator to pass as a --use argument to AutoRest.");
console.log("\t\tOption#2: The version of AutoRest.Java. E.g. 2.0.9. You can also pass latest or preview.");

@ChenTanyi Tanyi Chen (ChenTanyi) Dec 10, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think option should use same number of '\t'

@ChenTanyi Tanyi Chen (ChenTanyi) Dec 10, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about use another parameter --autorest-java-version rather than use same parameter?

console.log("\tUsually you'll only need to provide this and not a --autorest argument in order to work on Java code generation.");
console.log("\tSee https://github.com/Azure/autorest/blob/master/docs/developer/autorest-extension.md");
console.log("\tSee https://github.com/Azure/autorest/blob/master/.attic/developer/autorest-extension.md");

console.log("--debug");
console.log("\tFlag that allows you to attach a debugger to the autorest.java generator.");
Expand All @@ -62,15 +63,19 @@ var autoRestVersion = 'preview'; // default
if (args['autorest'] !== undefined) {
autoRestVersion = args['autorest'];
}
var autoRestJavaVersion = ''; // default
if (args['autorest-java'] !== undefined) {
autoRestJavaVersion = args['autorest-java'];
}
var debug = args['debug'];
var autoRestArgs = args['autorest-args'] || '';
var autoRestExe;
const mgmtPomFilename = 'pom.mgmt.xml'

gulp.task('codegen', function(cb) {
if (autoRestVersion.match(/[0-9]+\.[0-9]+\.[0-9]+.*/) ||
autoRestVersion == 'preview') {
autoRestExe = 'autorest ---version=' + autoRestVersion;
autoRestVersion == 'preview' || autoRestVersion == 'latest') {
autoRestExe = 'autorest --version=' + autoRestVersion;
handleInput(projects, cb);
} else {
autoRestExe = "node " + path.join(autoRestVersion, "src/autorest-core/dist/app.js");
Expand Down Expand Up @@ -107,9 +112,10 @@ var codegen = function(project, cb) {

console.log('Generating "' + project + '" from spec file ' + specRoot + '/' + mappings[project].source);

const generatorPath = args['autorest-java']
? `--use=${path.resolve(args['autorest-java'])} `
: '';
const generatorPath = autoRestJavaVersion == 'preview' || autoRestJavaVersion == 'latest'
|| autoRestJavaVersion.match(/[0-9]+\.[0-9]+\.[0-9]+.*/)

@ChenTanyi Tanyi Chen (ChenTanyi) Dec 10, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could match some path include version, like ~/test-1.2.3/***. May use ^ at first, but I think split into two parameters is better

@weidongxu-microsoft Weidong Xu (weidongxu-microsoft) Dec 10, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xccc-msft please consider tanyi's suggestion. I think use ^ would be fine.

If possible also print out the resolution of this line, if not already there. Then if anything went wrong caller would have log to see why.

? `--use=@microsoft.azure/autorest.java@` + autoRestJavaVersion +` `
: (autoRestJavaVersion == '' ? '' : `--use=${path.resolve(args['autorest-java'])} `);

const regenManager = args['regenerate-manager'] ? ' --regenerate-manager=true ' : '';

Expand All @@ -135,6 +141,7 @@ var codegen = function(project, cb) {
' --azure-arm ' +
' --azure-libraries-for-java-folder=' + sdkRoot + ' ' +
` --license-header=MICROSOFT_MIT_NO_CODEGEN ` +
` --use=@microsoft.azure/autorest.java@` + autoRestJavaVersion +` `
Comment thread
xseeseesee marked this conversation as resolved.
Outdated
generatorPath +
regenManager +
genInterface +
Expand Down