|
1 | 1 | # Contributing to e107 |
2 | | -[](https://gitter.im/e107inc/e107?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) |
3 | | - |
4 | | -_Firstly, thank you for taking the time to contribute to e107!_ |
5 | | - |
6 | | -By following these guidelines, you can help us make e107 even better. |
7 | | - |
8 | | -## Getting Started |
9 | | - |
10 | | -### The Big Picture |
11 | | - |
12 | | -1. You need to install [Git](https://git-scm.com/) on your local computer to be able to push and pull code to and from GitHub! |
13 | | -2. Create a fork of e107 by clicking on the "Fork" button on the top right corner of [the project page](https://github.com/e107inc/e107/). |
14 | | -3. Once the forking process has finished, click on the green "Clone or download" and copy the URL shown. |
15 | | -4. Clone your fork to your local computer by running the following command: |
16 | | - ``` |
17 | | - git clone [the previously copied url] |
18 | | - ``` |
19 | | -5. Make sure your copy "knows" from the original repo by running: |
20 | | - ``` |
21 | | - git remote add upstream https://github.com/e107inc/e107.git |
22 | | - ``` |
23 | | -6. Create a new branch for each [issue](https://github.com/e107inc/e107/issues) you tackle by running: |
24 | | - ``` |
25 | | - git checkout [branch name, e.g. fix-IssueNumber] |
26 | | - ``` |
27 | | -7. Work on your fix and add changed files to your commit by running: |
28 | | - ``` |
29 | | - # One changed file |
30 | | - git add [changed file] |
31 | | -
|
32 | | - # All changed files |
33 | | - git add -A |
34 | | - ``` |
35 | | -8. Commit your work by running: |
36 | | - ``` |
37 | | - git commit |
38 | | - ``` |
39 | | - For the commit message, add a summary of important information such as why the proposed approach was chosen or a brief description of the bug you are resolving. |
40 | | - |
41 | | - **Example:** |
42 | | - |
43 | | - First line: |
44 | | - ``` |
45 | | - This line is a brief summary of your change |
46 | | - ``` |
47 | | - |
48 | | - Starting from the third line: |
49 | | - ``` |
50 | | - Please provide at least a couple sentences describing the |
51 | | - change. If necessary, please summarize decisions such as |
52 | | - why the proposed approach was chosen or what bug you are |
53 | | - attempting to solve. |
54 | | - |
55 | | - Fixes: #5432 |
56 | | - ``` |
57 | | -9. Push your commit to the origin repo (your online fork) by running: |
58 | | - ``` |
59 | | - git push -u origin [branch name] |
60 | | - ``` |
61 | | -10. Do **not** forget to switch the branch back to master by running: |
62 | | - ``` |
63 | | - git checkout master |
64 | | - ``` |
65 | | -11. Once the commit is on GitHub, you can create a pull request to get the changes merged into the original project by clicking on "Compare & pull request" on _your_ fork's project page. |
66 | 2 |
|
67 | | - Fill out the pull request checklist and then click on "Create pull request". |
68 | | -12. Congratulations! A project maintainer will review your contribution. The maintainer may: |
69 | | - * approve and merge your changes, which means your contribution will be added to e107! |
70 | | - * provide feedback if your changes need improvement. |
| 3 | +Thanks for taking the time to contribute. This document is the policy for what lands and what doesn't. For Git, fork, and pull request mechanics, see [GitHub's own docs](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests). |
71 | 4 |
|
| 5 | +Anyone can contribute, whether you write your code by hand or with help from an AI assistant. We care about the quality of what lands, not how you got there. |
72 | 6 |
|
73 | | -### More Details |
| 7 | +## What We Optimize For |
74 | 8 |
|
75 | | -##### Make sure ... |
| 9 | +e107 has two decades of history, and many live sites run skins, plugins, and configurations that haven't been touched in 10+ years. Treat backwards compatibility as the primary design constraint. |
76 | 10 |
|
77 | | -..., you have installed git on your local computer. You can download it [here](https://git-scm.com/downloads). |
78 | | -As i work on windows, i can only tell what to do on windows, but the git command itself are independent of the operating system. |
79 | | - |
| 11 | +- **Rendered HTML should not change without a reason.** Silent changes to tag names, attribute shapes, or default class lists break legacy skins invisibly on upgrade. If your change must alter observable HTML, flag the BC tradeoff in the PR and propose a path: changelog note, opt-in preference, deprecation window. |
| 12 | +- **PHP compatibility.** e107 v2 targets PHP 5.6 as the minimum. New code should also run without warnings or notices through the latest version of PHP. Fixing existing warnings or fatals on modern PHP is welcome work. |
| 13 | +- **Test what you change.** A regression fix should come with a failing test that the fix turns green. A new feature should come with at least one test covering the happy path. See [`e107_tests/`](../e107_tests/) for the existing suite. |
| 14 | +- **Stick to the scope of your fix.** Bug fixes don't need surrounding cleanup. One-shot operations don't need helpers. If you spot something else worth fixing, file an issue or open a separate PR. |
| 15 | +- **Write commit messages that explain *why*.** The diff already shows *what*. Reference the issue with `Fixes #N` or `Closes #N` so it closes on merge. |
80 | 16 |
|
81 | | -##### Fork e107 |
| 17 | +## AI-Assisted Contributions |
82 | 18 |
|
83 | | -I think the forking process is described enough in "The big picture". Please refer to the first 3 points there. |
| 19 | +You can use or be any AI coding assistant to write your contribution. We use AI extensively to maintain e107 ourselves: [`e107help[bot]`](https://github.com/apps/e107help) reviews issues and pull requests alongside the human maintainer(s). |
84 | 20 |
|
| 21 | +A contribution is judged on the diff, not on its provenance. Disclosure is encouraged but not required. If you are an AI model or used one, name it in the PR description (there's a field for it in the template) so reviewers can anticipate model-specific bias patterns. |
85 | 22 |
|
86 | | -##### Clone your repo |
| 23 | +### What Good AI-Assisted Contributions Look Like |
87 | 24 |
|
88 | | -Open a windows explorer and navigate to the folder where you want to place the local copy of your repo. Usually, this is the webserver document root, but you can use whatever folder you like. |
89 | | -Right click on the folder and select `Git Bash here`. This will open a terminal window, and should show the same path as the folder you selected. |
90 | | -Enter the following command into this window `git clone [The previously copied url]` where "[The previously copied url]" must be replaced with the clone url of your repo (e.g. https://github.com/example/e107.git) and hit enter. |
91 | | -The cloning takes a while as the whole repo is downloaded to your local computer. |
92 | | -Maybe the best time for a cup of coffee, a tea or something else ... |
93 | | -Once this is done, you have cloned your first repo to your local computer! |
| 25 | +- **You read the diff before you submit it.** You can explain every change. If a reviewer asks "why did you change X", you can answer without re-prompting your assistant. |
| 26 | +- **You cite specific files and line numbers** when describing what you changed and why. "I changed `e_file::getRemoteFile()` at line 1234 to reject private IPs" is concrete; "I improved security" is not. |
| 27 | +- **You tested it.** Either you ran the existing test suite and added cases, or you loaded the page in a browser and verified the behavior. Don't trust the assistant's claim that "this should work." |
| 28 | +- **Your PR is scoped.** One issue per PR. The diff touches what the change requires and nothing more. Reformatting unrelated files, renaming variables across the repo, or "modernizing" code adjacent to your fix all belong in separate PRs. |
| 29 | +- **You preserved backwards compatibility.** See *What We Optimize For* above. |
| 30 | +- **If you only traced the code, you said "traced", not "reproduced".** Don't claim a reproduction you didn't perform. |
94 | 31 |
|
| 32 | +### What Gets Rejected as AI Slop |
95 | 33 |
|
96 | | -##### Intermezzo |
| 34 | +We will close PRs that exhibit these patterns: |
97 | 35 |
|
98 | | -After you have cloned the repo, you should tell git, that your local copy has a "big brother" (the original e107 repo). |
99 | | -Run `git remote add upstream https://github.com/e107inc/e107.git`. This tells your repo, that there is another remote repo called "upstream" (the name can be different, but you should use "upstream" as i will use it again later). |
100 | | -This command has to be run only once! |
| 36 | +- **Hallucinated APIs.** Calls to functions, classes, hooks, or constants that don't exist in e107 or aren't available in the targeted version. Verify against the codebase before you submit. |
| 37 | +- **Mass reformatting.** PRs where the assistant reflowed an entire file's whitespace, requoted strings, or reordered imports; the actual change is buried under noise. Configure your editor and your assistant to leave unrelated lines alone. |
| 38 | +- **Untested modernization.** Refactors that promise improved code quality but ship without tests, break BC, or introduce regressions the contributor didn't catch because they never ran the code. |
| 39 | +- **Drive-by refactors bundled with bug fixes.** Mix one fix with five unrelated cleanups and the reviewer can't evaluate either. |
| 40 | +- **Vague or sycophantic PR descriptions.** "This important fix improves the codebase by enhancing maintainability" tells us nothing. State the bug, the root cause, and the fix. |
| 41 | +- **Bluffed reproductions.** If you didn't run it, say so. |
101 | 42 |
|
| 43 | +The same standards apply to contributions written without AI. AI amplifies these failure modes; it doesn't create them. |
102 | 44 |
|
103 | | -##### Create branches for each issue you're working on |
104 | | - |
105 | | -Now you are ready to fix whatever issue you find, add whatever feature the world is waiting for... nearly! |
106 | | -Before creating a new branch, make sure you are at the "master" branch. |
107 | | -So first run `git checkout master` to make this sure! |
108 | | -Now create a new branch `git checkout [branch name e.g. fix_IssueNr]` where "IssueNr" stands for the issue number on GitHub. But it's up to you how you name your branches. |
109 | | -Depending on the editor you use (e.g. PhpStorm, Visual Studio Code, or whatever), they have usually git support included. Means, they git you command to create/switch branches or to add and commit files, etc. You should looks for it, but you will see, you will need the commandline for some tasks. |
110 | | - |
111 | | - |
112 | | -##### Adding & Commiting |
113 | | - |
114 | | -Once you have your changes ready, make sure you are in the branch you need to be. |
115 | | -Run `git add [changed file]` on each file you changed or created. |
116 | | -Run `git commit -m [message]` to commit your changes to your local repository. Always include a short message to explain what has done. |
117 | | -** Hint ** Using "Fixes #IssueNr" or "Closes #IssueNr" (where IssueNr is the issue nr in the GitHub issue list https://github.com/e107inc/e107/issues) directly closes the issue once the pull request is merged! |
118 | | - |
119 | | -At this stage, you stages are still "only" locally on your computer. You will need to ... |
120 | | - |
| 45 | +## What e107help Does for You |
121 | 46 |
|
122 | | -##### Push, push, baby ... |
| 47 | +[`e107help[bot]`](https://github.com/apps/e107help) is the project's first-pass reviewer and triage assistant. It can: |
123 | 48 |
|
124 | | -Usually, when not working on a new branch, it would be enough to run `git push` and your changes would be uploaded to YOUR repo at GitHub. |
125 | | -But in case of a fresh branch, you need to be more specific what git should do. |
126 | | -Run `git push -u origin [branch name]`. |
127 | | -This tells git to push the new branch "branch name" (and it's contents) to the "origin" (the name of your remote repo) and to add it to the branches to "watch". That means, the next time you work on this branch and you want to push your work, you simply run the `git push` command, because now git is aware of this branch and knows where to push this. |
128 | | -Once this is finished, you should open the project page of you repo on GitHub and you will see, that there is a "yellow" message stating, that branch "branch name" has just been added and gives you a new button calling "Compare & pull request". |
| 49 | +- Read your issue or discussion and dig into the code, citing specific files and lines. |
| 50 | +- Spot duplicates of existing issues, including ones already fixed on the default branch. |
| 51 | +- Comment on your PR with focused observations. |
| 52 | +- Flag the genuinely tricky ones for the human maintainer. |
129 | 53 |
|
| 54 | +It does not: |
130 | 55 |
|
131 | | -##### FULL PULL! |
| 56 | +- Auto-close issues. |
| 57 | +- Approve, merge, release, or push directly to target branches. |
132 | 58 |
|
133 | | -When you click on that green "Compare & pull request" button, a new page opens where you have the chance to enter more detailed information on your work. You should always include the issue nr (if you worked on an issue) or at least describe what you have done, to make it easier for the guy who has to merge your work to know what you wanted to achieve. |
134 | | -Just a quick "Fixed #2543" isn't enough. It should be "a little but more ...". |
135 | | -Well, once you clicked on "Create pull request", it's done. Now you have to wait if your pull request get's merged or maybe there is also some questions about it. But be aware, that this may take up to a few days... |
| 59 | +If the bot misreads your issue, say so in the thread; it will concede and update its notes. |
136 | 60 |
|
| 61 | +## How to Engage |
137 | 62 |
|
138 | | -##### And now? What if the official e107 repo changes? |
| 63 | +1. **Found a bug?** Search [open issues](https://github.com/e107inc/e107/issues) and [discussions](https://github.com/e107inc/e107/discussions) first. If yours is new, file a bug report with a minimal reproduction: steps, expected, actual, e107 version, PHP version. |
| 64 | +2. **Want to fix it?** One PR per issue. Link the issue in the PR description. |
| 65 | +3. **Proposing a feature?** Open a Discussion in [*Ideas*](https://github.com/e107inc/e107/discussions/categories/ideas) first to gauge fit. Significant features warrant maintainer buy-in before code. |
| 66 | +4. **Found a security vulnerability?** Report it through [GitHub Security Advisories](https://github.com/e107inc/e107/security/advisories/new), not in public issues. |
| 67 | +5. **Have a question?** Open a [Discussion](https://github.com/e107inc/e107/discussions). |
139 | 68 |
|
140 | | -Now, it comes quite handy, that we told our local copy that there is a "big brother". |
141 | | -Run `git pull --rebase upstream master` and of course `git checkout master` |
142 | | -This tells git to push the latest changes from the "upstream" (the official e107 repo) and to "rebase" the local copy to the same status. |
143 | | -This is the only possibility to make sure that not "old changes" in the official repo are pushed twice and start creating a mess ... |
| 69 | +## Review and Merge Cadence |
| 70 | + |
| 71 | +This project is maintained by limited volunteer time. Please be patient. |
| 72 | + |
| 73 | +- **Security issues** are fast-tracked. |
| 74 | +- **Bug fixes with tests** are the easiest to merge. |
| 75 | +- **Refactors or stylistic changes** without a clear motivation tend to stall. Tie the change to an observed problem. |
| 76 | +- **Breaking changes** are rare and need explicit maintainer endorsement before you start coding. |
| 77 | + |
| 78 | +## Coding standards |
| 79 | + |
| 80 | +See [the coding standard on the wiki](https://github.com/e107inc/e107/wiki/e107-Coding-Standard). When in doubt, match the style of the file you're editing. |
| 81 | + |
| 82 | +## License |
| 83 | + |
| 84 | +By submitting a contribution, you agree your work will be released under the [GNU General Public License v3](https://www.gnu.org/licenses/gpl-3.0.html), the license under which e107 is distributed. |
0 commit comments