Skip to content

Remove unnecessary 2nd call to SwaggerParser.#2977

Closed
pablograna wants to merge 2 commits intoswagger-api:masterfrom
pablograna:master
Closed

Remove unnecessary 2nd call to SwaggerParser.#2977
pablograna wants to merge 2 commits intoswagger-api:masterfrom
pablograna:master

Conversation

@pablograna
Copy link
Copy Markdown
Contributor

Both CodeGenMojo and CodegenConfigurator where parsin the swagger input.
The result in CodeGenMojo was discarded. I simply removed the line in
CodeGenMojo.

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented May 27, 2016

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/swagger-api/swagger-codegen/graphs/contributors.

Let me know if you need help fixing it.

Ref: http://stackoverflow.com/questions/3042437/change-commit-author-at-one-specific-commit

Both CodeGenMojo and CodegenConfigurator where parsin the swagger input.
The result in CodeGenMojo was discarded. I simply removed the line in
CodeGenMojo.
Eclipse was thrownig some errors related to differences in package names
vs file path. I renamed the packages io.swagger.codegen.Go to
io.swagger.codegen.go, io.swagger.codegen.slim to package
io.swagger.codegen.lumen and io.swagger.codegen.springBoot to
io.swagger.codegen.springboot in some test files.

I also made some parameters final, I hope you don't mind. The coding
conventions said nothing about final parameters.
@pablograna
Copy link
Copy Markdown
Contributor Author

I think I've fixed it, now the page shows the commits.

case "multi": return "(QueryList 'MultiParamArray (" + type + "))";
default:
throw new NotImplementedException();
throw new UnsupportedOperationException();
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 suggest we keep NotImplementedException as existing programs may be a dependency on this. If we switch it to UnsupportedOperationException, we'll need to flag this change as breaking change, which seems not necessary.

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented May 28, 2016

Yes, I think you've fixed it as confirmed in the commit tabs.

Btw, I would suggest you to create a new branch for the change as per Git best practice.

@pablograna
Copy link
Copy Markdown
Contributor Author

Here is the 'clean' PR, just the line removed in a new branch:

#3005

Should I close this PR?

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.

2 participants