sylvainlaurent / swagger-validator-maven-plugin

A maven plugin that validates swagger files in yaml and json formats
Apache License 2.0
11 stars 6 forks source link

Issue 19 #20

Closed oyeli closed 5 years ago

giuliopulina commented 5 years ago

I see that there is a merge commit in the list of commit (https://github.com/sylvainlaurent/swagger-validator-maven-plugin/pull/20/commits/2fec17606bca97d346d7e91d6ea13639a53545c3) could you please check?

giuliopulina commented 5 years ago

@oyeli to me it looks good. Could you please just check my previous comment:

I see that there is a merge commit in the list of commit (2fec176) could you please check?

oyeli commented 5 years ago

@oyeli to me it looks good. Could you please just check my previous comment:

I see that there is a merge commit in the list of commit (2fec176) could you please check?

I've just updated my fork from your repo using pull request. There is no changes actually. Is there another way how we can do it? If there is such possibility you can squash it.

giuliopulina commented 5 years ago

@oyeli to me it looks good. Could you please just check my previous comment:

I see that there is a merge commit in the list of commit (2fec176) could you please check?

I've just updated my fork from your repo using pull request. There is no changes actually. Is there another way how we can do it? If there is such possibility you can squash it.

Hi @oyeli, I just downloaded your fork and it seems that commit 2fec176 is present both on your master branch and Issue-19 branch, but it's not something I would merge in upstream/master: like you said, this merge commits contains no changes, but it would make the history 'dirty'.

I think that the best way to solve the issue is to rebase your Issue-19 from upstream/master (and then push force on oyeli/Issue-19), I already did it, just for testing, in my fork and you can see the result here

oyeli commented 5 years ago

@oyeli to me it looks good. Could you please just check my previous comment:

I see that there is a merge commit in the list of commit (2fec176) could you please check?

I've just updated my fork from your repo using pull request. There is no changes actually. Is there another way how we can do it? If there is such possibility you can squash it.

Hi @oyeli, I just downloaded your fork and it seems that commit 2fec176 is present both on your master branch and Issue-19 branch, but it's not something I would merge in upstream/master: like you said, this merge commits contains no changes, but it would make the history 'dirty'.

I think that the best way to solve the issue is to rebase your Issue-19 from upstream/master (and then push force on oyeli/Issue-19), I already did it, just for testing, in my fork and you can see the result here

Thanks for your suggestion! I've done it. Looking forward to the next version which will include my changes.

giuliopulina commented 5 years ago

@oyeli to me it looks good. Could you please just check my previous comment:

I see that there is a merge commit in the list of commit (2fec176) could you please check?

I've just updated my fork from your repo using pull request. There is no changes actually. Is there another way how we can do it? If there is such possibility you can squash it.

Hi @oyeli, I just downloaded your fork and it seems that commit 2fec176 is present both on your master branch and Issue-19 branch, but it's not something I would merge in upstream/master: like you said, this merge commits contains no changes, but it would make the history 'dirty'. I think that the best way to solve the issue is to rebase your Issue-19 from upstream/master (and then push force on oyeli/Issue-19), I already did it, just for testing, in my fork and you can see the result here

Thanks for your suggestion! I've done it. Looking forward to the next version which will include my changes.

You're welcome :) For merge and new release, we need to wait for @sylvainlaurent

kh0ma commented 5 years ago

Hello @sylvainlaurent

Any updates here? We are looking forward to seeing it in the next version.

sylvainlaurent commented 5 years ago

hello @kh0ma , release 1.2.6 includes this PR