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

PHPDoc Cleanup #438

Closed
erayd opened this issue Jun 23, 2017 · 15 comments
Closed

PHPDoc Cleanup #438

erayd opened this issue Jun 23, 2017 · 15 comments

Comments

@erayd
Copy link
Contributor

erayd commented Jun 23, 2017

The current codebase lacks useful PHPDoc comments in many places, and needs a significant overhaul.

I would like to propose the following:

  • Every class and interface must have:
    • A short description of what the class
    • A long description explaining the class in more depth if appropriate
    • @package justinrainbow/json-schema
    • @license MIT
  • Every method must have:
    • A short description of the method
    • A long description explaining the method in more depth if appropriate
    • @api if the method is part of the official public API
    • @param <type> <name> for all parameters; mixed-type params should be stated as mixed
    • @return <type> [description] for all methods which return a value. The return description should be present for any method where the return value is not self-evident from reading the method description.
  • Every abstract method signature should be documented as per fully-defined methods.
  • Every property must have:
    • @var <type> <description>
  • Every class constant must have a PHPDoc description, but should not contain any @tags.

I would also like to add a config file for sami/sami, and delete the old documentation directory - docs hasn't been touched in 5+ years, and doesn't contain anything of substance anyway. Sami should not be added as a dev-time dependency, as it requires PHP >= 7.0 to run.

Before I go ahead and sink time into this, does what I've outlined above sound reasonable?

Finally, if someone else is keen and has the time, feel free to jump in with a PR. I'm quite happy to do all the work on this, but documentation isn't my favourite activity, so if you happen to harbour a secret passion for it...

@bighappyface
Copy link
Collaborator

Sounds good to me

@shmax
Copy link
Collaborator

shmax commented Jun 23, 2017

Sure, sounds good. I recently went through the whole process of using Sami to deploy its output to github pages for a different project, so I can help with travis if you'd like to do something similar.

@erayd
Copy link
Contributor Author

erayd commented Jun 23, 2017

@shmax That would definitely be nice if you're willing to do that again here :-).

@shmax
Copy link
Collaborator

shmax commented Jun 23, 2017

Can do, but we'll need someone with access to the settings to configure github pages. @bighappyface?

@erayd
Copy link
Contributor Author

erayd commented Jun 23, 2017

Oh. If that needs admin access, then we're SOL on that front, because @bighappyface isn't, and @justinrainbow is gone.

@shmax
Copy link
Collaborator

shmax commented Jun 23, 2017

Ugh. Well, we could always deploy it somewhere else. Any ideas?

@erayd
Copy link
Contributor Author

erayd commented Jun 23, 2017

I have a webserver on DigitalOcean we can put this on, if that's a viable option. It has various PHP websites on it, but nothing too heavy-duty, and there's a fair bit of spare capacity on there.

@shmax
Copy link
Collaborator

shmax commented Jun 23, 2017

That could work. We don't even need PHP; just anything that can host static pages. I've only deployed to github, but I've been meaning to learn how to do other destinations, so this will be interesting. Should this work happen independently of your PHPDoc stuff, or in concert?

@erayd
Copy link
Contributor Author

erayd commented Jun 23, 2017

Independently I reckon. Stand by, and I'll add a user account to the server for you.

@shmax
Copy link
Collaborator

shmax commented Jun 23, 2017

Take your time, I'm off to bed. Should be able to dabble with this a bit on Sunday.

@erayd
Copy link
Contributor Author

erayd commented Jun 23, 2017

Already done it :-). See Slack.

@bighappyface
Copy link
Collaborator

^^ nvm

@erayd
Copy link
Contributor Author

erayd commented Jun 27, 2017

What do you guys think should be done with the @author tags? Are they still relevant, noting how many people have contributed to this library over the years?

If we keep them, how do we determine who is tagged - do we just tag everyone who has current code in the file where the tag appears, and not care about the list being huge?

Personally, I think it makes more sense to have a CONTRIBUTORS or AUTHORS file in the repo root that lists contributors, and remove the @author tags completely, as they get unweildy when there are a large number of contributors.

@DannyvdSluijs
Copy link
Collaborator

Looking at this issue I've found a couple of things which might impact the original report.

  • Sami has archived as of February 2019 (over 5 years ago)
  • Back in 2017 PHP 7.2 was not released and @param and @return were the default way of documenting the functions etc. Since the language and eco system have evolved with parameter and return types as well as tools like PHPStan and Rector.
  • With regards to the @author tags, I'm not a big believer of code owners of individual files but would rather see the whole as a community effort, with the commit history to show for it. In addition the library is under MIT license and therefor the claiming of being an author has no value as well (to my knowledge).

A short and simple permissive license with conditions only requiring preservation of copyright and license notices. Licensed works, modifications, and larger works may be distributed under different terms and without source code.

  • We now know that time available from the maintainers is limited (no judgment here, just pointing out an observation)

With all of the above I would opt for dropping the issue and spend time more wisely.

@erayd
Copy link
Contributor Author

erayd commented Apr 11, 2024

@DannyvdSluijs Sounds sensible; I agree with your opinion on this.

@erayd erayd closed this as completed Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants