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

False warning for put/patch when using resources macro #1

Open
woylie opened this issue May 30, 2017 · 3 comments
Open

False warning for put/patch when using resources macro #1

woylie opened this issue May 30, 2017 · 3 comments
Labels

Comments

@woylie
Copy link
Member

woylie commented May 30, 2017

Phoenix.Router.resources defines both a PATCH route and a PUT route for the :update operation. In your controller, you will only have one function for both routes, and that means, you will only define api/3 for either PATCH or PUT. This will lead BlueBird to emit a warning about missing documentation for the other method.

@woylie woylie added the bug label May 30, 2017
@woylie
Copy link
Member Author

woylie commented Jun 2, 2017

I don't think it's possible to get intel about whether a route was defined via the resources macro. That leaves us with three options:

  1. We leave it as it is and tell our users to ignore the warnings. I wouldn't like that.
  2. We check whether a PATCH route also has a PUT route which was described with the api/3 macro and vice versa. If so, we suppress the warning. In this case, we might have false negatives if an api really handles PATCH and PUT differently. It's not ideal, but at least we get helpful warnings in most cases without annoying users with false warnings.
  3. We remove the warnings altogether. But I found those warnings helpful already, so I'd like to keep them.

What do you think?

@rhazdon
Copy link
Member

rhazdon commented Jun 2, 2017

I don't like to suppress or ignore warnings either. But we should be able to determine if a route for :put also exists for :patch and vice versa. What do you think about creating the docs for the "undocumented", but yet existing route, automatically? Provided the route is tested.

@woylie
Copy link
Member Author

woylie commented Jun 3, 2017

So let's implement option 2 for now.

As for creating docs for routes without documentation: It would be possible, of course, but we would also have to extract the parameters automatically, because otherwise they wouldn't be displayed in the generated docs. That would make the use of the macros optional. In that case, the warnings should be removed.

This is a fundamental question. Do we want a package that requires explicit documentation and helps you find gaps in your application? Or do we want a package that builds a basic documentation for you out-of-the-box, with the option of adding more descriptions and information as needed, but potentially leading you to treat your documentation half-heartedly?

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

No branches or pull requests

2 participants