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

Gen.go update #547

Merged
merged 3 commits into from
Feb 16, 2017
Merged

Gen.go update #547

merged 3 commits into from
Feb 16, 2017

Conversation

mcardosos
Copy link
Contributor

This works with the cowboy's latest changes.
PTAL @marstr @jhendrixMSFT

Copy link
Member

@marstr marstr left a comment

Choose a reason for hiding this comment

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

Small-ish comments, good practice though. :)

Gododir/gen.go Outdated
@@ -404,14 +414,16 @@ func generate(service *service) {
fmt.Printf("Generating %s...\n\n", service.Fullname)
delete(service)

autorest := exec.Command(fmt.Sprintf("%s/autorest/src/core/AutoRest/bin/Debug/net451/win7-x64/autorest", autorestDir),
autorest := exec.Command("gulp",
Copy link
Member

@marstr marstr Feb 15, 2017

Choose a reason for hiding this comment

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

Given that this is pretty new stuff, and some of our computers (okay, well mine) haven't had gulp setup yet, we may want to add in some guard rails like this:
exec.LookPath(string)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Gododir/gen.go Outdated
"-Input", fmt.Sprintf("%s/azure-rest-api-specs/%s.json", swaggersDir, service.Input),
"-CodeGenerator", "Go",
"-Header", "MICROSOFT_APACHE",
"-Namespace", service.Name,
"-OutputDirectory", service.Output,
"-Modeler", "Swagger",
"-pv", sdkVersion)
autorest.Dir = fmt.Sprintf("%s/autorest", autorestDir)
Copy link
Member

Choose a reason for hiding this comment

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

We're all cross-platform like now! How about we update it to:

autorest.Dir = filepath.Join(autores.Dir, "autorest")

filepath.Join(string...)

Gododir/gen.go Outdated
}

autorest := exec.Command("gulp",
"autorest",
"-Input", fmt.Sprintf("%s/azure-rest-api-specs/%s.json", swaggersDir, service.Input),
Copy link
Member

Choose a reason for hiding this comment

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

Another candidate for filepath.Join.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed (and used filepath.Join in some other places too)

@mcardosos
Copy link
Contributor Author

mcardosos commented Feb 15, 2017

Wait, it is not working as I expected yet :I
Do not merge

@mcardosos
Copy link
Contributor Author

It works!

@mcardosos mcardosos merged commit f8e2ba3 into Azure:dev Feb 16, 2017
@mcardosos mcardosos deleted the gen branch February 16, 2017 18:55
marstr pushed a commit to marstr/azure-sdk-for-go that referenced this pull request Mar 9, 2017
* Updated Gen.go to use new autorest builld system

* Comments from review

* It works as expected again
marstr added a commit that referenced this pull request Mar 31, 2017
* Running latest Go Generator on all released packages.

* Gen.go update (#547)

* Updated Gen.go to use new autorest builld system

* Comments from review

* It works as expected again

* Moving Storage to an Independent Repository (#556)

* Updating CI to use Go 1.8.

Go 1.8 was released on Thursday, February 16th. Many of our partners
have adopted and we need to ensure that we support the most recent
version.

* Replacing storage folder with submodule.

In order to allow for more independent growth of the storage package,
we've given it its own repository. This change updates the main
repository to use a submodule in order to prevent breaking changes for
folks using storage today who do not want to migrate to the new
repository.

* Updating submodule target URL.

There is an existing naming convention for Azure Storage repositories.
the repo has been updated to match that convention, this updates the
submodule to point to the new URL.

* Responding to review feedback.

* Updating submodule to ref v8.1.0 storage version.

* Fixing issues found while reviewing PR.

* Regenerating using latest swaggers.

Using Azure/azure-rest-api-specs commit: 7d9b6c0

* Regenerating with new API Version strategy & web composite.

The new API Version strategy associated the API Version to send across the wire with each method instead of at the client level. Doing so allows us to support a broader range of composite swaggers (the immediate example is the web composite swagger.)

It also means that making a call with an alternate API version is somewhat more difficult in this version. To do so, one would have to create their own Preparer for the request. In the future, we'll support multiple API Versions and allow for more stability by generating independent packages for each Operation Group in each API Version. We expect to have that done in the summer.

* Regenerating against most recent swaggers.

* Regenerating using latest preview version of compute.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants