From ab24f596606742a60fe81c482bc5714d07385aa7 Mon Sep 17 00:00:00 2001 From: Diogo Cordeiro Date: Sun, 2 Jun 2019 20:36:47 +0100 Subject: [PATCH] [DOCUMENTATION] Add CONTRIBUTING information for developers Inspired both from GNU FM, postActiv and Moodle --- .../DEVELOPERS/CONTRIBUTING/README.md | 198 +++++++++++++ .../DEVELOPERS/CONTRIBUTING/boilerplate.php | 61 ++++ .../CONTRIBUTING/coding_standards.md | 275 ++++++++++++++++++ .../CONTRIBUTING/merge_request_checklist.md | 32 ++ 4 files changed, 566 insertions(+) create mode 100644 DOCUMENTATION/DEVELOPERS/CONTRIBUTING/README.md create mode 100644 DOCUMENTATION/DEVELOPERS/CONTRIBUTING/boilerplate.php create mode 100644 DOCUMENTATION/DEVELOPERS/CONTRIBUTING/coding_standards.md create mode 100644 DOCUMENTATION/DEVELOPERS/CONTRIBUTING/merge_request_checklist.md diff --git a/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/README.md b/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/README.md new file mode 100644 index 0000000000..1c7d0d9c15 --- /dev/null +++ b/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/README.md @@ -0,0 +1,198 @@ +# Contributing to GNU social + +First of all, if you're reading this intending to contribute to GNU social, +thanks! Free software development only happens when people like you take an +interest in giving back to the software they themselves use, and their +community. + +When contributing to this repository, please first discuss the change you wish to +make via issue, email, or any other method with the owners of this repository before +making a change. + +There's a few files you should read before going forward with a merge request +or a patch submission. They detail what this file touches on in brief. They +are: + +* coding_standards.md: How your code should be structured and formatted to be + accepted into the GNU social codebase. +* merge_request_checklist.md: A quick checklist to review before submission. + +## Merge Request Process + +1. Ensure you strip any trailing spaces off and checked the file with php-cs-fixer +2. Increase the version numbers in any examples files and the README.md to the new version that this + Pull Request would represent. The versioning scheme we use is [SemVer](http://semver.org/). +3. You may merge the Pull Request in once you have the sign-off of two other developers, or if you + do not have permission to do that, you may request the second reviewer to merge it for you. + + +## Code of Conduct + +### Our Pledge + +In the interest of fostering an open and welcoming environment, we as +contributors and maintainers pledge to making participation in our project and +our community a harassment-free experience for everyone, regardless of age, body +size, disability, ethnicity, gender identity and expression, level of experience, +nationality, personal appearance, race, religion, or sexual identity and +orientation. + +### Our Standards + +Examples of behavior that contributes to creating a positive environment +include: + +* Using welcoming and inclusive language +* Being respectful of differing viewpoints and experiences +* Gracefully accepting constructive criticism +* Focusing on what is best for the community +* Showing empathy towards other community members + +Examples of unacceptable behavior by participants include: + +* The use of sexualized language or imagery and unwelcome sexual attention or +advances +* Trolling, insulting/derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or electronic + address, without explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +### Our Responsibilities + +Project maintainers are responsible for clarifying the standards of acceptable +behavior and are expected to take appropriate and fair corrective action in +response to any instances of unacceptable behavior. + +Project maintainers have the right and responsibility to remove, edit, or +reject comments, commits, code, wiki edits, issues, and other contributions +that are not aligned to this Code of Conduct, or to ban temporarily or +permanently any contributor for other behaviors that they deem inappropriate, +threatening, offensive, or harmful. + +### Scope + +This Code of Conduct applies both within project spaces and in public spaces +when an individual is representing the project or its community. Examples of +representing a project or community include using an official project e-mail +address, posting via an official social media account, or acting as an appointed +representative at an online or offline event. Representation of a project may be +further defined and clarified by project maintainers. + +### Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported by contacting the project team at [INSERT EMAIL ADDRESS]. All +complaints will be reviewed and investigated and will result in a response that +is deemed necessary and appropriate to the circumstances. The project team is +obligated to maintain confidentiality with regard to the reporter of an incident. +Further details of specific enforcement policies may be posted separately. + +Project maintainers who do not follow or enforce the Code of Conduct in good +faith may face temporary or permanent repercussions as determined by other +members of the project's leadership. + +### Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], version 1.4, +available at [http://contributor-covenant.org/version/1/4][version] + +[homepage]: http://contributor-covenant.org +[version]: http://contributor-covenant.org/version/1/4/ + + +## The Code of Conflict + +GNU social has a high submission standard and we want to keep quality code in the +codebase and bad code out of it. As such your code will be closely scrutinized, +and you might take this criticism personally. Please understand that this is +meant to keep the standards of the codebase up, and isn't meant personally. All +the same, this isn't an excuse for poor behaviour, and a reviewer shouldn't be +misbehaving towards submitters. The Code of Conflict outlines the dispute +resolution mechanism if something does come up, so give it a read. + + +## Coding Standards + +Since we will be expected to maintain your code once it's submitted, we ask you +to adhere to certain coding standards that make it easier for us to do so. If +code doesn't follow them, it will be rejected, so please read up on these. + + +## Bug Reports + +Please report bugs to the issue tracker at + Avoid assigning the labels +yourself, as these are for the development team to assign priority and area of +coverage to a subject. Please only submit something here if you are certain it +is a bug or represents a feature enhancement that we do not presently have. If +you are uncertain whether it's a bug, please feel free to ask +at #social IRC channel on freenode.net https://www.freenode.net/. + +When reporting a bug, please try to include as much information as possible, +including the environment being run on (if it's a common LAMP stack just give +us version numbers of the main stack components, that's fine), and the specific +error you get. If you do not get a client-facing error, please check the PHP +error_log and ensure there isn't something silently reported there, as well as +the GNU social log. Try to include steps to reproduce the error as well, as if +we cannot reproduce the error, we can't fix it! + +It is perfectly acceptable to reference the archive page of a discussion on the +mailing list for the bug report, by the way, as long as it includes all the +information we need for a bug report. + + +## Submitting Feature Requests / Enhancement Requests + +Social media is constantly evolving, and we welcome ideas about how we can +change and evolve GNU social to keep it the excellent piece of software that it +is. However, there are a few things we ask you do when submitting feature +requests: + +1. Understand that since we have a limited amount of developers and these people +contribute in their free time, we may prioritize things differently than you +value them. Oftentimes this is because certain requests involve less changes +to the existing codebase than others, and therefore this makes them easier +to add. +2. Please search the existing feature requests and enhancements to see if a +similar request exists. If one does but you have different ideas about how +to do it or what it should entail, please add a comment to the existing idea +rather than create a new one for your "version" of it. Duplicate submissions +mean we spend more time maintaining the tracker and less time actually +working on the codebase! +3. When outlining the way that you see something working, don't be afraid to be +as detailed as possible! We may not implement it exactly as you describe for +any variety of reasons, but the more concrete and fleshed out an idea is, the +easier it is for us to know what you want and be able to implement it in a +sane and secure fashion. +4. When describing a possible new idea and its mechanisms of operation, the key +words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", +"SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in the issue submission +are to be interpreted as described in RFC 2119. + + +Finally, and just as a call back to the first point, realize just because we +might not rush to implement something, doesn't mean that we don't want to +implement it! We would rather take the time to do something right the first +time, then hurriedly apply a new idea, or a fix, only to have to patch it later. + + +## Branch of Code Submissions + +Unless you've been specifically directed otherwise, all submissions of code +should be against the `nightly` branch, so make sure any modifications are based +on Nightly. + + +## Copyright / Licensing + +You acknowledge that by submitting code to GNU social, you are licensing it under +the GNU AGPLv3 unless there is an extenuating circumstance where it would be +licensed differently (such as modifications to an external library we include +such as Stomp). + +You also acknowledge that unless you assign a copyright explicitly, it will be +assumed to be assigned to GNU social. + +Thanks for considering submission, and happy hacking! diff --git a/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/boilerplate.php b/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/boilerplate.php new file mode 100644 index 0000000000..3825a1c8b4 --- /dev/null +++ b/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/boilerplate.php @@ -0,0 +1,61 @@ +. + +/** + * Description of this file. + * + * @package samples + * @author Diogo Cordeiro + * @copyright 2019 Free Software Foundation, Inc http://www.fsf.org + * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later + */ + +namespace samples; + +defined('GNUSOCIAL') || die(); + +require_once(__DIR__ . DIRECTORY_SEPARATOR . 'SampleHandler.php'); + +/** + * Description of this class. + * + * @copyright 2019 Free Software Foundation, Inc http://www.fsf.org + * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later + */ +class MySampleClass +{ + /** + * Constructor for the sample class. + * + * @param string $dummy_word just because. + * @param int $result another just because. + */ + public function __construct(string $dummy_word = '', int $result = null) + { + global $demo; + $this->niceWorld(); + } + + /** + * How cool is this function. + * + * @return string + */ + public function niceWorld() : string + { + return 'hello, world.'; + } +} diff --git a/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/coding_standards.md b/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/coding_standards.md new file mode 100644 index 0000000000..988680e6b3 --- /dev/null +++ b/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/coding_standards.md @@ -0,0 +1,275 @@ +GNU social Coding Style +=========================== + +Please comply with [PSR-2](https://www.php-fig.org/psr/psr-2/) and the following standard when working on GNU social +if you want your patches accepted and modules included in supported releases. + +If you see code which doesn't comply with the below, please fix it :) + + +Strings +------------------------------------------------------------------------------- +Use `'` instead of `"` for strings, where substitutions aren't required. +This is a performance issue, and prevents a lot of inconsistent coding styles. +When using substitutions, use curly braces around your variables - like so: + + $var = "my_var: {$my_var}"; + + +Comments and Documentation +------------------------------------------------------------------------------- +Comments go on the line ABOVE the code, NOT to the right of the code, unless it is very short. +All functions and methods are to be documented using PhpDocumentor - https://docs.phpdoc.org/guides/ + +File Headers +------------------------------------------------------------------------------- +File headers follow a consistent format, as such: + + // This file is part of GNU social - https://www.gnu.org/software/social + // + // GNU social is free software: you can redistribute it and/or modify + // it under the terms of the GNU Affero General Public License as published by + // the Free Software Foundation, either version 3 of the License, or + // (at your option) any later version. + // + // GNU social is distributed in the hope that it will be useful, + // but WITHOUT ANY WARRANTY; without even the implied warranty of + // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + // GNU Affero General Public License for more details. + // + // You should have received a copy of the GNU Affero General Public License + // along with GNU social. If not, see . + + /** + * Description of this file. + * + * @package samples + * @author Diogo Cordeiro + * @copyright 2019 Free Software Foundation, Inc http://www.fsf.org + * @license https://www.gnu.org/licenses/agpl.html GNU AGPL v3 or later + */ + +Please use it. + +A few notes: + +* The description of the file doesn't have to be exhaustive. Rather it's + meant to be a short summary of what's in this file and what it does. Try + to keep it to 1-5 lines. You can get more in-depth when documenting + individual functions! + +* You'll probably see files with multiple authors, this is by + design - many people contributed to GNU social or its forebears! If you + are modifying an existing file, APPEND your own author line, and update + the copyright year if needed. Do not replace existing ones. + +You may find `boilerplate.php` useful when creating a new file from scratch. + +Paragraph spacing +------------------------------------------------------------------------------- +Where-ever possible, try to keep the lines to 80 characters. Don't +sacrifice readability for it though - if it makes more sense to have it in +one longer line, and it's more easily read that way, that's fine. + +With assignments, avoid breaking them down into multiple lines unless +neccesary, except for enumerations and arrays. + + +'If' statements format +------------------------------------------------------------------------------- +Use switch statements where many else if's are going to be used. Switch/case is faster + + if ($var == 'example') { + echo 'This is only an example'; + } else { + echo 'This is not a test. This is the real thing'; + } + +Do NOT make if statements like this: + + if ($var == 'example'){ echo 'An example'; } + + OR this + + if($var = 'example') + echo "An {$var}"; + + +Associative arrays +------------------------------------------------------------------------------- +Always use `[]` instead of `array()`. Associative arrays must be written in the +following manner: + + $array = [ + 'var' => 'value', + 'var2' => 'value2' + ]; + +Note that spaces are preferred around the '=>'. + + +A note about shorthands +------------------------------------------------------------------------------- +Some short hands are evil: + +- Use the long format for `post($contents); + +Well I can easily tell what you're doing there because the names are straight- +forward and clear. + +Something like this: + + foo->bar(); + +Is much less clear. + +Also, whereever possible, avoid ambiguous terms. For example, don't use text +as a term for a variable. Call back to "contents" above. + + +Comparisons +------------------------------------------------------------------------------- +Always use symbol based comparison operators (&&, ||) instead of text based +operators (AND, OR) as they are evaluated in different orders and at different +speeds. This is will prevent any confusion or strange results. + + +Use English +------------------------------------------------------------------------------- +All variables, classes, methods, functions and comments must be in English. +Bad english is easier to work with than having to babelfish code to work out +how it works. + + +Encoding +------------------------------------------------------------------------------- +Files should be in UTF-8 encoding with UNIX line endings. + + +No ending tag +------------------------------------------------------------------------------- +Files should not end with an ending php tag "?>". Any whitespace after the +closing tag is sent to the browser and cause errors, so don't include them. + + +Nesting Functions +------------------------------------------------------------------------------- +Avoid, if at all possible. When not possible, document the living daylights +out of why you're nesting it. It's not always avoidable, but PHP 5 has a lot +of obscure problems that come up with using nested functions. + +If you must use a nested function, be sure to have robust error-handling. +This is a must and submissions including nested functions that do not have +robust error handling will be rejected and you'll be asked to add it. + + +Scoping +------------------------------------------------------------------------------- +Properly enforcing scope of functions is something many PHP programmers don't +do, but should. + +In general: +* Variables unique to a class should be protected and use interfacing to + change them. This allows for input validation and making sure we don't have + injection, especially when something's exposed to the API, that any program + can use, and not all of them are going to be be safe and trusted. + +* Variables not unique to a class should be validated prior to every call, + which is why it's generally not a good idea to re-use stuff across classes + unless there's significant performance gains to doing so. + +* Classes should protect functions that they do not want overriden, but they + should avoid protecting the constructor and destructor and related helper + functions as this prevents proper inheritance. + + +Typecasting +------------------------------------------------------------------------------- +PHP is a soft-typed language and it falls to us developers to make sure that +we are using the proper inputs. Where ever possible use explicit type casting. +Where it in't, you're going to have to make sure that you check all your +inputs before you pass them. + +All outputs should be cast as an explicit PHP type. + +Not properly typecasting is a shooting offence. Soft types let programmers +get away with a lot of lazy code, but lazy code is buggy code, and frankly, I +don't want it in GNU social if it's going to be buggy. + + +Consistent exception handling +------------------------------------------------------------------------------- +Consistency is key to good code to begin with, but it is especially important +to be consistent with how we handle errors. GNU social has a variety of built- +in exception classes. Use them, wherever it's possible and appropriate, and +they will do the heavy lifting for you. + +Additionally, ensure you clean up any and all records and variables that need +cleanup in a function using try { } finally { } even if you do not plan on +catching exceptions (why wouldn't you, though? That's silly.) + +If you do not call an exception handler, you must, at a minimum, record errors +to the log using common_log(level, message) + +Ensure all possible control flows of a function have exception handling and +cleanup, where appropriate. Don't leave endpoints with unhandled exceptions. +Try not to leave something in an error state if it's avoidable. + + +Return values +------------------------------------------------------------------------------- +All functions must return a value. Every single one. This is not optional. + +If you are simply making a procedure call, for example as part of a helper +function, then return boolean TRUE on success, and the exception on failure. + +When returning the exception, return the whole nine yards, which is to say the +actual PHP exception object, not just an error message. + +All return values not the above should be type cast, and you should sanitize +anything returned to ensure it fits into the cast. You might technically make +an integer a string, for instance, but you should be making sure that integer +SHOULD be a string, if you're returning it, and that it is a valid return +value. + +A vast majority of programming errors come down to not checking your inputs +and outputs properly, so please try to do so as best and thoroughly as you can. + + +Layout and Location of files +------------------------------------------------------------------------------- +`/actions/` contains files that determine what happens when something "happens": +for instance, when someone favourites or repeats a notice. Code that is +related to a "happening" should go here. + +`/classes/` contains abstract definitions of certain "things" in the codebase +such as a user or notice. If you're making a new "thing", it goes here. + +`/lib/` is basically the back-end. Actions will call something in here to get +stuff done usually, which in turn will probably manipulate information stored +in one or more records represented by a class. + +`/extlib/` is where external libraries are located. If you include a new +external library, it goes here. + +`/plugins/` This is a great way to modularize your own new features. If you want +to create new core features for GNU social, it is probably best to create a +module unless you absolutely must override or modify the core behaviours. diff --git a/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/merge_request_checklist.md b/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/merge_request_checklist.md new file mode 100644 index 0000000000..e26c209dcf --- /dev/null +++ b/DOCUMENTATION/DEVELOPERS/CONTRIBUTING/merge_request_checklist.md @@ -0,0 +1,32 @@ +Submission Checklist +================================================================================ +This document serves as a handy checklist for submitted merges and patches to +the postActiv project. Following it isn't a gaurantee a patch will be accepted, +but it will help you avoid common problems. + +1. Ensure all code control paths in all functions return a value. + +2. Ensure all exceptions are trapped in an exception class, or minimally, + written to the log with common_log + +3. Ensure the coding format standards are adhered to (see coding_standards.md) + +4. Ensure that any new class that deals in public data has a corresponding new + API endpoint. + +5. Ensure that all new API endpoints sanitize inputs and outputs properly. + +6. Ensure that your version of the code works with PHP 7 on a standard + LAMP and LEMP stack (Linux+Apache+MariaDB+PHP and Linux+nginx+MariaDB+PHP) + +7. If implementing new database functions, ensure they work with MariaDB + and postgreSQL. + +8. Ensure all data that federates does so properly and has mechanisms to + catch and accomodate for federation transmission failure. + +9. Ensure that nothing is left in an error state when it is avoidable. + +10. Ensure that all code submitted is properly documented. + +11. Ensure that there are no PHP Strict Standards or Parse errors in the code.