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

Recreate swagger.json in each travis build #462

Merged
merged 1 commit into from Aug 29, 2018

Conversation

@xeviknal
Copy link
Member

commented Aug 27, 2018

** Describe the change **

I have found the following scenario:

  1. User1 adding invalid go-swagger comments into a field.
  2. User1 not calling make swagger-generate
  3. User1 getting merged his/her PR.
  4. User2 adding go-swagger comments into a completely different feature.
  5. User2 calling make swagger-generate swagger-validate and getting multiple errors unrelated to his/her feature.

I think we have to prevent user to push swagger-code that is not validated. Here, my proposal to fix it.
It has to be safe to re-create swagger.json in each build because any developer should modify that file. It has to be done always via go-swagger comments.

@xeviknal xeviknal requested review from aljesusg and lucasponce Aug 27, 2018
Copy link
Contributor

left a comment

LGFM

@xeviknal xeviknal force-pushed the xeviknal:travis-swagger-validate branch from 7bbc556 to bc4f194 Aug 28, 2018
@josejulio

This comment has been minimized.

Copy link
Member

commented Aug 28, 2018

but we are only creating and not committing this file?

If we want to be more strict, maybe we can check in travis if the generated file is exactly the same as the latest copy (ensuring that the dev comited and pushed their swagger.json)

@lucasponce

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2018

I understand that this change lets travis to perform an additional validation and it will catch more missings issues in the doc; I also like @josejulio comments that perhaps we should be more strict.
All comments are complementary and it can be done in following interations.

Copy link
Contributor

left a comment

LGTM
@josejulio has valid points but I guess we can address his suggestion in a following PR too.

@aljesusg

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2018

I tried to do that months ago @josejulio tthe travis swagger file generated is ever different that the swagger file that you commit, we need some special check for this. I'll investigate about it.

@lucasponce lucasponce merged commit 2cc6e82 into kiali:master Aug 29, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@xeviknal xeviknal deleted the xeviknal:travis-swagger-validate branch Aug 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.