mirror of
https://git.eden-emu.dev/eden-emu/eden
synced 2026-04-10 03:18:55 +02:00
[docs] reorg a bit, add AI policy, rewrite release policy (#3423)
mdlint, AI policy, and an actual release policy that doesn't suck. Signed-off-by: crueter <crueter@eden-emu.dev> Reviewed-on: https://git.eden-emu.dev/eden-emu/eden/pulls/3423 Reviewed-by: Lizzie <lizzie@eden-emu.dev> Reviewed-by: DraVee <dravee@eden-emu.dev>
This commit is contained in:
parent
979ea5563b
commit
a692986bd7
9 changed files with 157 additions and 180 deletions
107
docs/policies/AI.md
Normal file
107
docs/policies/AI.md
Normal file
|
|
@ -0,0 +1,107 @@
|
|||
# AI Policy
|
||||
|
||||
Use at your peril.
|
||||
|
||||
AI is a *tool*, not a replacement or catch-all solution. It is generally okay at a few *very specific* use cases:
|
||||
|
||||
- Automation of tedious changes where you have already made the pattern clear and done the necessary groundwork.
|
||||
- Conversion of code from one paradigm to another.
|
||||
|
||||
For everything else, AI is subpar at best, and actively harmful at worst. In general, you are **heavily** encouraged to not use AI at all.
|
||||
|
||||
## Why?
|
||||
|
||||
AI is notorious for hallucinating facts out of thin air and sometimes outright lying to users. Additionally, code written by LLMs is often needlessly verbose and horrifically inefficient (not to mention the rather ridiculous level of over-commenting). The end result is often one of three things:
|
||||
|
||||
- Completely nonfunctional code
|
||||
- Code that works, but is extraordinarily verbose or not nearly as efficient as it can be
|
||||
- Code that works well and is written well, but solves a different problem than was intended, or solves the same problem but in a completely incorrect way that will break other things horribly.
|
||||
|
||||
Human-written code will, without exception, always be of infinitely higher quality when properly researched and implemented by someone both familiar with the surrounding code and the programming language in use. LLMs may produce a "good enough" result, but this result is often subpar. Keep in mind: all code is held under a standard of excellence. If your code sucks, it will be rejected. AI-generated code just so happens to be a particularly sucky genre of code, and will thus be held to this same standard.
|
||||
|
||||
On a lesser-known note, LLM outputs often contain unicode symbols such as emojis or the arrow symbol. Please don't put Unicode symbols in your code. It messes with many an IDE, and the three people viewing your code on Lynx will be very unhappy.
|
||||
|
||||
**Learn to code**. It's worth it, we promise!
|
||||
|
||||
## Acceptable Use
|
||||
|
||||
- As stated previously, AI is good in a few *very specific* cases. In these cases, it's usually fine to use AI, as long as you **explicitly provide notice that it was used**.
|
||||
- Anything directly outside of the realm of the code written in your PR or patch is none of our business.
|
||||
- This primarily covers research.
|
||||
- However, we *still* strongly discourage this for the reasons mentioned above.
|
||||
- Assistance with cleanups, and minor nitpicks/optimizations.
|
||||
- This is still discouraged, but it's okay to occasionally use LLMs to catch any minor mistakes you made in your code.
|
||||
- Debugging
|
||||
- In general, LLMs are not good at debugging, but if you solve a bug you're facing with help from an AI, and said fix **works properly**, then that's fine.
|
||||
|
||||
## Unacceptable Use
|
||||
|
||||
Well, everything else. But here are a few examples:
|
||||
|
||||
- Commit messages
|
||||
- LLMs are absolutely horrible at this. They are needlessly verbose, almost never catch the actual intent of the commit, and will almost always hallucinate false information about said changes.
|
||||
- See the addendum for an example
|
||||
- Solving problems
|
||||
- Slapping a few files and a "please fix bug XYZ" into an LLM is a recipe for disaster that will pretty much never work.
|
||||
- Fully AI-generated code
|
||||
- This shouldn't need explaining. Do not do this under any circumstance, especially if you don't actually understand what's going on.
|
||||
- Writing code based on pseudo-instructions
|
||||
- If you don't know how to write code, don't. If you've figured out the root cause (preferably without feeding random files into an LLM) and actively know what's going on, provide information to other developers or friends of yours who have knowledge of the language and/or the codebase.
|
||||
|
||||
## Addendum: Commit Messages
|
||||
|
||||
The patchset for pull request [#3422](https://git.eden-emu.dev/eden-emu/eden/pulls/3422) was fed into several LLMs to generate a commit message. All of them sucked, and not a single one caught on to what the commit actually did. For example:
|
||||
|
||||
```txt
|
||||
profile_manager: Refactor user deletion to use index instead of UUID
|
||||
|
||||
This commit improves the user profile deletion flow in the following ways:
|
||||
|
||||
1. **UI layer (ConfigureProfileManager)**:
|
||||
- Replaces UUID-based deletion with index-based deletion
|
||||
- Passes the selected row index from the tree view to the confirmation dialog
|
||||
- Emits deleteUser(int index) signal from ConfigureProfileManagerDeleteDialog
|
||||
- Removes lambda capture and std::function callback in favor of direct signal/slot connection
|
||||
|
||||
2. **Delete dialog**:
|
||||
- Changes SetInfo() signature to accept index instead of accept callback
|
||||
- Stores index internally and emits it via deleteUser signal on acceptance
|
||||
|
||||
3. **Core ProfileManager**:
|
||||
- Refactors RemoveUser(UUID) → RemoveProfileAtIndex(index)
|
||||
- Removes duplicated stable_partition + save logic from RemoveUser()
|
||||
- Centralizes profile removal and save-file writing in RemoveProfileAtIndex()
|
||||
|
||||
4. **Minor follow-up**:
|
||||
- Adds profile_manager.ResetUserSaveFile() call in PopulateUserList() to ensure consistent state when refreshing the user list
|
||||
|
||||
These changes eliminate redundant code, reduce the risk of UUID → index mismatch bugs, and make the deletion path more straightforward and maintainable.
|
||||
|
||||
Fixes: potential inconsistency when user order changes between UI and backend
|
||||
```
|
||||
|
||||
This is:
|
||||
|
||||
- Needlessly verbose (nobody cares about most of these details)
|
||||
- Doesn't address the actual purpose of the PR (fixing a double-deletion bug in the profile manager)
|
||||
- Has unicode arrows
|
||||
- Uses overly corporate and, well, robotic language
|
||||
|
||||
As another example:
|
||||
|
||||
```txt
|
||||
profile(manager/ui): switch to index-based deletion and unify removal logic
|
||||
|
||||
- Make ConfigureProfileManagerDeleteDialog emit a deleteUser(int) signal instead of taking a callback. Store the target index in the dialog (m_index) and emit it on acceptance.
|
||||
- Change ConfigureProfileManager to pass an index to SetInfo and to accept an index in DeleteUser. Wire the dialog signal to the new DeleteUser slot.
|
||||
- Update headers: add the signal and m_index, add TODO to move dialog to a .ui file, and update slot/signature declarations.
|
||||
- Add ProfileManager::RemoveProfileAtIndex(std::size_t) and refactor RemoveUser(UUID) to call RemoveProfileAtIndex to avoid duplicated removal logic. Ensure the removal path marks saves as needed and writes the user save file.
|
||||
- Ensure the profile list updates immediately after deletes by calling profile_manager.ResetUserSaveFile() when populating the user list (qlaunch fix).
|
||||
- Misc: update SPDX copyright year and fix build breakages caused by the API changes.
|
||||
|
||||
This consolidates profile removal behavior, fixes potential race conditions in the profile dialog, and removes duplicated removal code.
|
||||
```
|
||||
|
||||
This has all of the same problems as the other one. Needlessly verbose, doesn't address *what* it actually fixes ("consolidates profile removal behavior"... okay, why? What does it fix?), etc. It even has the bonus of totally hallucinating the addition of a method!
|
||||
|
||||
**Don't use AI for commit messages**.
|
||||
126
docs/policies/Coding.md
Normal file
126
docs/policies/Coding.md
Normal file
|
|
@ -0,0 +1,126 @@
|
|||
# Coding guidelines
|
||||
|
||||
These are mostly "suggestions", if you feel like your code is readable, comprehensible to others; and most importantly doesn't result in unreadable spaghetti you're fine to go.
|
||||
|
||||
But for new developers you may find that following these guidelines will make everything x10 easier.
|
||||
|
||||
## Naming conventions
|
||||
|
||||
Simply put, types/classes are named as `PascalCase`, same for methods and functions like `AddElement`. Variables are named `like_this_snake_case` and constants are `IN_SCREAMING_CASE`.
|
||||
|
||||
Except for Qt MOC where `functionName` is preferred.
|
||||
|
||||
Template typenames prefer short names like `T`, `I`, `U`, if a longer name is required either `Iterator` or `perform_action` are fine as well. Do not use names like `SS` as systems like solaris define it for registers, in general do not use any of the following for short names:
|
||||
|
||||
- `SS`, `DS`, `GS`, `FS`: Segment registers, defined by Solaris `<ucontext.h>`
|
||||
- `EAX`, `EBX`, `ECX`, `EDX`, `ESI`, `EDI`, `ESP`, `EBP`, `EIP`: Registers, defined by Solaris.
|
||||
- `X`: Defined by some utility headers, avoid.
|
||||
- `_`: Defined by gettext, avoid.
|
||||
- `N`, `M`, `S`: Preferably don't use this for types, use it for numeric constants.
|
||||
- `TR`: Used by some weird `<ucontext.h>` whom define the Task Register as a logical register to provide to the user... (Need to remember which OS in specific).
|
||||
|
||||
Macros must always be in `SCREAMING_CASE`. Do not use short letter macros as systems like Solaris will conflict with them; a good rule of thumb is >5 characters per macro - i.e `THIS_MACRO_IS_GOOD`, `AND_ALSO_THIS_ONE`.
|
||||
|
||||
Try not using hungarian notation, if you're able.
|
||||
|
||||
## Formatting
|
||||
|
||||
Formatting is extremelly lax, the general rule of thumb is: Don't add new lines just to increase line count. The less lines we have to look at, the better. This means also packing densely your code while not making it a clusterfuck. Strike a balance of "this is a short and comprehensible piece of code" and "my eyes are actually happy to see this!". Don't just drop the entire thing in a single line and call it "dense code", that's just spaghetti posing as code. In general, be mindful of what other devs need to look at.
|
||||
|
||||
Do not put if/while/etc braces after lines:
|
||||
|
||||
```c++
|
||||
// no dont do this
|
||||
// this is more lines of code for no good reason (why braces need their separate lines?)
|
||||
// and those take space in someone's screen, cumulatively
|
||||
if (thing)
|
||||
{ //<--
|
||||
some(); // ...
|
||||
} //<-- 2 lines of code for basically "opening" and "closing" an statment
|
||||
|
||||
// do this
|
||||
if (thing) { //<-- [...] and with your brain you can deduce it's this piece of code
|
||||
// that's being closed
|
||||
some(); // ...
|
||||
} //<-- only one line, and it's clearer since you know its closing something [...]
|
||||
|
||||
// or this, albeit the extra line isn't needed (at your discretion of course)
|
||||
if (thing)
|
||||
some(); // ...
|
||||
|
||||
// this is also ok, keeps things in one line and makes it extremely clear
|
||||
if (thing) some();
|
||||
|
||||
// NOT ok, don't be "clever" and use the comma operator to stash a bunch of statments
|
||||
// in a single line, doing this will definitely ruin someone's day - just do the thing below
|
||||
// vvv
|
||||
if (thing) some(), thing(), a2(a1(), y1(), j1()), do_complex_shit(wa(), wo(), ploo());
|
||||
// ... and in general don't use the comma operator for "multiple statments", EXCEPT if you think
|
||||
// that it makes the code more readable (the situation may be rare however)
|
||||
|
||||
// Wow so much clearer! Now I can actually see what each statment is meant to do!
|
||||
if (thing) {
|
||||
some();
|
||||
thing();
|
||||
a2(a1(), y1(), j1());
|
||||
do_complex_shit(wa(), wo(), ploo());
|
||||
}
|
||||
```
|
||||
|
||||
Brace rules are lax, if you can get the point across, do it:
|
||||
|
||||
```c++
|
||||
// this is fine
|
||||
do {
|
||||
if (thing) {
|
||||
return 0;
|
||||
}
|
||||
} while (other);
|
||||
|
||||
// this is also ok --- albeit a bit more dense
|
||||
do if (thing) return 0; while (other);
|
||||
|
||||
// ok as well
|
||||
do {
|
||||
if (thing) return 0;
|
||||
} while (other);
|
||||
```
|
||||
|
||||
There is no 80-column limit but preferably be mindful of other developer's readability (like don't just put everything onto one line).
|
||||
|
||||
```c++
|
||||
// someone is going to be mad due to this
|
||||
SDL_AudioSpec obtained;
|
||||
device_name.empty() ? device = SDL_OpenAudioDevice(nullptr, capture, &spec, &obtained, false) : device = SDL_OpenAudioDevice(device_name.c_str(), capture, &spec, &obtained, false);
|
||||
|
||||
// maybe consider this
|
||||
SDL_AudioSpec obtained;
|
||||
if (device_name.empty()) {
|
||||
device = SDL_OpenAudioDevice(nullptr, capture, &spec, &obtained, false);
|
||||
} else {
|
||||
device = SDL_OpenAudioDevice(device_name.c_str(), capture, &spec, &obtained, false);
|
||||
}
|
||||
|
||||
// or this is fine as well
|
||||
SDL_AudioSpec obtained;
|
||||
device = SDL_OpenAudioDevice(device_name.empty() ? nullptr : device_name.c_str(), capture, &spec, &obtained, false);
|
||||
```
|
||||
|
||||
A note about operators: Use them sparingly, yes, the language is lax on them, but some usages can be... tripping to say the least.
|
||||
|
||||
```c++
|
||||
a, b, c; //<-- NOT OK multiple statments with comma operator is definitely a recipe for disaster
|
||||
return c ? a : b; //<-- OK ternaries at end of return statments are clear and fine
|
||||
return a, b; //<-- NOT OK return will take value of `b` but also evaluate `a`, just use a separate statment
|
||||
void f(int a[]) //<-- OK? if you intend to use the pointer as an array, otherwise just mark it as *
|
||||
```
|
||||
|
||||
And about templates, use them sparingly, don't just do meta-templating for the sake of it, do it when you actually need it. This isn't a competition to see who can make the most complicated and robust meta-templating system. Just use what works, and preferably stick to the standard libary instead of reinventing the wheel. Additionally:
|
||||
|
||||
```c++
|
||||
// NOT OK This will create (T * N * C * P) versions of the same function. DO. NOT. DO. THIS.
|
||||
template<typename T, size_t N, size_t C, size_t P> inline void what() const noexcept;
|
||||
|
||||
// OK use parameters like a normal person, don't be afraid to use them :)
|
||||
template<typename T> inline void what(size_t n, size_t c, size_t p) const noexcept;
|
||||
```
|
||||
10
docs/policies/Release.md
Normal file
10
docs/policies/Release.md
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
# Release Policy
|
||||
|
||||
Release when lots of new changes and fixes. Hotfix if more bugs. Release candidate if lot of things to test. Simple as.
|
||||
|
||||
## Checklist
|
||||
|
||||
- [ ] Update Transifex
|
||||
- [ ] Test for regressions and bugs
|
||||
- [ ] Write a changelog
|
||||
- [ ] Ensure all platforms work
|
||||
Loading…
Add table
Add a link
Reference in a new issue