search

GetShopTV / swagger2

Swagger 2.0 data model.
http://hackage.haskell.org/package/swagger2
BSD 3-Clause "New" or "Revised" License
74 stars 59 forks source link

Better validation errors #195

Closed fisx closed 4 years ago

fisx commented 5 years ago

Fixes https://github.com/GetShopTV/swagger2/issues/140

Stolen from servant-swagger

fisx commented 5 years ago

thanks @sphaso!

fisx commented 5 years ago

any comments on this? it would be nice to get this merged, since it also improves on the error messages provided in servant-swagger slightly (it also dumps the descriptions on the swagger docs).

thanks!

fisx commented 5 years ago

ping...

fisx commented 5 years ago

just a friendly reminder that i'm still interested in this. @phadej? or perhaps somebody else has the authority to decide that i've answered all his requests?

fisx commented 5 years ago

Under what conditions would it be possible for me to become a co-maintainer? I am interested in using this library professionally, but I would have to either fork it or make it somehow easier for changes to happen here.

fizruk commented 5 years ago

@fisx Apologies for late responses!

Right now I don't have enough time lately to properly maintain this package (I have all the functionality I need for my projects, so maintaining is low priority). Apparently, @phadej also has his plate full. So I think we would only benefit from with adding you as a maintainer here and, probably, on Hackage 😊

phadej commented 5 years ago

There's also a conflict...

phadej commented 5 years ago

FYI: I dislike comments with commit message Fixup, so if @fizruk gives @fisx a maintainer bits, we'd need to agree a bit more rules as there would be more cooks in the kitchen (i.e. write them down).

fisx commented 5 years ago

FYI: I dislike comments with commit message Fixup, so if @fizruk gives @fisx a maintainer bits, we'd need to agree a bit more rules as there would be more cooks in the kitchen (i.e. write them down).

I am used to squashing during merge, so I'm sometimes not too concerned with commit messages. If you don't want to allow squash-merge in this repo, I would like to go through my branch again and clean it up a little.

In general, of course I'm willing to follow your rules if you are nice enough to let me work with you (even though I'll argue :)). I suggest we write down whatever comes to mind, and I'll leave all PRs open a minimum amount of time to give you time to add more rules as the need arises?

Thanks for the fast reply (this time)! :)

fisx commented 5 years ago

Thanks for inviting me to mess with this project, I will try very hard not to disappoint you! :-)

I will let this PR sit for your final approval / more change requests for another 5 days. If I don't hear from you, I will assume your approval, merge, and make a github release. I would prefer though to get a nod from you @fizruk and @phadej on this policy in general, at least this time.

And I'm happy to do the hackage release as well if you let me in on that, too. About that: I have tried to guess the way versions work in this project in the last commit: it's a non-breaking change, but it's not just a revision either, so 2.4 becomes 2.4.1. Does that make sense to everybody?

fizruk commented 5 years ago

@fisx I would replace 5 days with 2 weeks, but the rule seems okay to me otherwise.

Regarding versioning: we are sticking with PVP for this package, so act accordingly. Either way, release process (including version bump and changelog updates) takes a separate PR, so don't do it here.

GetShop.TV coding style is available here. For commit messages please refer to a blog post by Chris Beams on «How to Write a Good Commit Message». I am typically not in favour of squash merge, but I do not have strong opinions against it.

Meanwhile I will try add more people from @GetShopTV to join in as co-maintainers to make it easier to get things reviewed.

P.S. Can you write me at nickolay at getshoptv dot com to discuss other details?

fisx commented 5 years ago

@fisx I would replace 5 days with 2 weeks, but the rule seems okay to me otherwise.

Ok, two weeks start now. I will merge either on Oct 22, or any time before that if I have your approval.

Regarding versioning: we are sticking with PVP for this package, so act accordingly. Either way, release process (including version bump and changelog updates) takes a separate PR, so don't do it here.

Ok, I have removed the commit.

GetShop.TV coding style is available here. For commit messages please refer to a blog post by Chris Beams on «How to Write a Good Commit Message». I am typically not in favour of squash merge, but I do not have strong opinions against it.

I like both articles a lot, thanks! I think I've done ok before even knowing them, but I've found some things I haven't thought of so far that I want to incorporate into my style.

Meanwhile I will try add more people from @GetShopTV to join in as co-maintainers to make it easier to get things reviewed.

:+1:

P.S. Can you write me at nickolay at getshoptv dot com to discuss other details?

done!

thanks :)