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

Deprecated parameter handling #140

Open
Clorith opened this issue Oct 20, 2014 · 7 comments
Open

Deprecated parameter handling #140

Clorith opened this issue Oct 20, 2014 · 7 comments

Comments

@Clorith
Copy link
Member

Clorith commented Oct 20, 2014

Per now the devhub doesn't play well with deprecated parameters for function calls, an example of this would be the get_the_author function which used to have a parameter.

The parser currently picks these up as Required, which is a little bit unlucky.

Deprecated parameters seem to consistently be defined with @param $var Deprecated, so detecting these shouldn't be an issue, but how should we relay that to the user in a clear manner, and in such a way that it doesn't accidentally get read as the whole function being deprecated which the current use of $deprecated as the parameter name might lead to.

@Rarst
Copy link
Contributor

Rarst commented Oct 20, 2014

Two separate issues:

  1. Required is clearly a bug, probably unrelated to deprecated.
  2. Deprecated detection for these is tricky. _deprecated_argument() call has no connection with which argument is deprecated.

@Clorith
Copy link
Member Author

Clorith commented Oct 20, 2014

For the 2nd bit here, if the Deprecated use in the inline docs is as consistent at it seems to be (i'm sure @DrewAPicture can chime in if this is so) then that would be the ideal thing to match against I'd think?

@DrewAPicture
Copy link
Member

It's trivial to determine whether an entire function is deprecated, and that's not in contention here.

As @Rarst correctly pointed out, marking them as Required is clearly a bug, so it sounds like maybe the Optional keyword hasn't been consistently applied. To adjust our standard we need to propose the change at the inline docs chat on Wednesday, but I can see probably doing an audit for all deprecated arguments and coming up with a standard for how to properly mark them up.

Probably the most consistent way would be to add the Deprecated. keyword at the beginning of the description and treat that as Optional (which it is). Once we've made a decision, I can do a quick sweep through core and fix (likely) all of the deprecated argument descriptions at once.

@JDGrimes
Copy link
Contributor

The best way to determine whether an argument is optional or required is to check for its default value. If it doesn't have one, then it is required.

@Rarst
Copy link
Contributor

Rarst commented Oct 20, 2014

It's trivial to determine whether an entire function is deprecated, and that's not in contention here.

Yeah. the tricky part is deprecated argument in a non–deprecated function.

@DrewAPicture
Copy link
Member

Yeah. the tricky part is deprecated argument in a non–deprecated function.

Yeah, see the entire paragraph I wrote about what we'll need to do to audit core and update the standards specific to deprecated arguments :)

@Rarst
Copy link
Contributor

Rarst commented Oct 20, 2014

Not picking on you, just keeping the focus clear in the thread. :) We had topics wander away before. :D

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

No branches or pull requests

4 participants