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

Update Q&A based on feedback from users #695

Merged
merged 3 commits into from
Aug 5, 2018
Merged

Update Q&A based on feedback from users #695

merged 3 commits into from
Aug 5, 2018

Conversation

wing328
Copy link
Member

@wing328 wing328 commented Jul 31, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

(details of the change, additional tests that have been done, reference to the issue for tracking, etc)

Copy link
Contributor

@tedepstein tedepstein left a comment

Choose a reason for hiding this comment

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

Much better. I have made a couple of other minor suggestions, which I think can tighten it up a bit, and complete the "depersonalization" of your concerns. You can take these suggestions or not, as you see fit.

docs/qna.md Outdated
1. The founding members came to the conclusion that Swagger Codegen 3.0.0 beta contains too many breaking changes while they strongly believe 3.0.0 release should only focus on one thing: OpenAPI 3.0 support.
1. We had concerns about the development practices, which seemed to be contributing to an unstable and insufficiently tested codebase.
1. We could not agree on an efficient evolutionary strategy. The founding members felt it was important to move forward with OpenAPI 3.0 support, while maintaining backward compatibility with OpenAPI 2.0 in the same codebase.
1. We found that the enhancements, bug fixes submitted for Swagger Codegen 2.x need to be submitted again for Swagger Codegen 3.0.0 branch. Otherwise, these changes will not appear in the 3.0.0 branch. Having to do the PR twice is not the best use of the community resource.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this just a consequence of the previous point? If so, consider leaving it out.

If you think the consequence is important, maybe just append to the previous point: "...in the same codebase, rather than maintaining two separate branches in parallel."

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the feedback. It may or may not be the consequence of the previous point and that's why I think we should elaborate with a separate point.

In OpenAPI Generator, we maintain different branches for patch, minor and major (e.g. 4.0.x with breaking chagnes) releases and we regularly sync changes between these branches to reduce community effort in submitting duplicated PRs.

In other words, maintaining different branches for the project to evolve is perfectly fine but not having a practice/workflow to sync enhancements, bug fixes between branches is less than ideal.

(These are my opinions)

docs/qna.md Outdated
1. We could not agree on an efficient evolutionary strategy. The founding members felt it was important to move forward with OpenAPI 3.0 support, while maintaining backward compatibility with OpenAPI 2.0 in the same codebase.
1. We found that the enhancements, bug fixes submitted for Swagger Codegen 2.x need to be submitted again for Swagger Codegen 3.0.0 branch. Otherwise, these changes will not appear in the 3.0.0 branch. Having to do the PR twice is not the best use of the community resource.
1. The community-driven version has a much more rapid [release cycle](https://github.com/OpenAPITools/openapi-generator/releases/) (weekly patch release, monthly minor release) so users do not need to wait for several months to get a stable release.
1. Having a community-driven version can bring the project to the next level with reliable releases and a clear [roadmap](https://github.com/OpenAPITools/openapi-generator/blob/master/docs/roadmap.adoc).

#### Has anything been done in attempt to address the issues before deciding to fork Swagger Codegen and maintain a community-driven version?

Copy link
Contributor

Choose a reason for hiding this comment

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

The following line mentions "SmartBear (Ron, Hugo)." I would suggest leaving Ron and Hugo's names out of this, again to make it less personal. You could just leave it as "SmartBear" or "the project owners" or "the other project owners" (if some of you were also owners).

@jmini
Copy link
Member

jmini commented Aug 2, 2018

I have pushed a commit based on other feedback I got. Changes are:

  • Put "What is the difference between Swagger Codegen and OpenAPI Generator?" as second question
  • Remove "we", change it with "founding members":

you are talking about ‘we’. Just reading this there is no clue as to who ‘we’ are.

  • Other small fixes

@wing328
Copy link
Member Author

wing328 commented Aug 3, 2018

Thanks @jmini

If no further feedback from anyone, I'll merge it into the master over the weekend.

@wing328 wing328 merged commit c9085b4 into master Aug 5, 2018
@wing328 wing328 deleted the update_qna branch August 5, 2018 09:49
@wing328
Copy link
Member Author

wing328 commented Aug 5, 2018

PR merged into master. Thanks for all the review and feedback.

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
* update qna based on feedback from users

* Improvements

* minor fix to typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants