Core merge 17112025 (#104)
* fix(DB/SmartAI): improve Harry surrendering during quest 'Gambling Debt' (#23598) * fix(DB/Quest): The Kalu'ak dailies reward 500 rep (#23600) * chore(DB): import pending files Referenced commit(s):fb03f41b2a* fix(DB/GameEvent): Remove midsummer pole in K3 (#23614) * chore(DB): import pending files Referenced commit(s):7b0000d6ee* fix(DB/SmartAI): increase reliability of quest event Foolish Endeavors (#23612) * chore(DB): import pending files Referenced commit(s):86f219abbc* fix(Scripts/AreaTrigger): players become stuck after Last Rites (#23613) * chore(DB): import pending files Referenced commit(s):c1a8047cf1* fix(Core/Vmaps): Fix inconsistency of hitInstance and hitModel to cause wrong area ids (#23233) Co-authored-by: ModoX <moardox@gmail.com> Co-authored-by: Shauren <shauren.trinity@gmail.com> Co-authored-by: Grimdhex <237474256+Grimdhex@users.noreply.github.com> Co-authored-by: sudlud <sudlud@users.noreply.github.com> * fix(DB/Gameobject): Sniffed Values for 'Wild Mustard' spawns (#23608) * fix(DB/SmartAI): remove large combat distance of Frostbrood Sentry (#23607) * chore(DB): import pending files Referenced commit(s):41d40b236f* fix(DB/ReputationRewardRate): Patch 3.0.0 gain for Northrend factions (#23597) * chore(DB): import pending files Referenced commit(s):067a898caa* fix(Core/Map): It should be ensured that the instance is unloaded only after the Creature Respawn. (#23103) * fix(Scripts/Northrend): Sniffing Out The Perpetrator horde (#23620) * fix(Scripts/Northrend): ensure Drakuru stays in place during Betrayal (#23619) * chore(DB): import pending files Referenced commit(s):928e145694* fix(DB/SmartAI): quest 'Reconnaissance Flight' (#23628) Co-authored-by: dr-j <dr-j@users.noreply.github.com> Co-authored-by: Killyana <morphone1@gmail.com> * fix(DB/QuestOfferReward): remove mention of a beta recipe in text (#23629) * fix(DB/Conditions): update quest conditions to drop chokers (#23610) * chore(DB): import pending files Referenced commit(s):bca8f7ce07* refactor(Core/PlayerScript): Delete OnPlayerChat, use OnPlayerCanUseChat (#23617) * fix(Core/SmartAI): startup warnings unused params (#23551) * fix(Core/Unit): Druid Talent Survival of the Fittest lacking immunity to creature daze (#23471) * fix(DB/SAI): Fix Fizzcrank Paradrop teleporters (#23633) * chore(DB): import pending files Referenced commit(s):94ba1c210d* fix(Core): Fix waterwalking after dying in instance (#23593) * fix(DB/SAI): don't remove all auras when mounting flamebringer (#23640) * chore(DB): import pending files Referenced commit(s):22f91f3802* fix(DB/SAI): Emerald Lasher goes out of the terrain when aggroed. (#23642) * chore(DB): import pending files Referenced commit(s):f9d6fe41de* fix(DB/SAI): Burning Depths Necromancer no longer stays in place. (#23641) * chore(DB): import pending files Referenced commit(s):1037471c8d* fix(DB/SAI): Remove SmartAI from Valkyrion Harpoon Gun. (#23646) * chore(DB): import pending files Referenced commit(s):8e3a7e6dcf* fix(DB/Creature): Fix Weakened Reanimated Frost Wyrm inhabit type (#23645) * chore(DB): import pending files Referenced commit(s):3baa18ef5b* fix(DB/Spell): Infectious Bites should stack from different casters (#23647) * chore(DB): import pending files Referenced commit(s):5aede412ab* fix(DB/SAI): Solve various issues with It Goes to 11... quest. (#23651) * fix(DB/Loot): Fireproof Satchel will now always drop the Ritual of Torch (#23585) * chore(DB): import pending files Referenced commit(s):1090c209b3* fix(Scripts/Northrend): Betrayal quest (#23650) * fix(Script/BlackTemple): Reliquary of Souls will use 45 degree in front to set incombat (#22938) * fix(Scripts/Spell): Fix Animal Blood spawning when it shouldn't (#23656) * fix(Scripts/BoreanTundra): Script Bloodspore Haze/Psychosis (#23657) * chore(DB): import pending files Referenced commit(s):baf7957e36* fix(DB/SAI): Sibling Rivalry quest credit if mounted (#23659) * chore(DB): import pending files Referenced commit(s):6919cc679d* fix(docs/license): use GPLv2 as MaNGOS-based project (#23655) * fix(Core/Achievements): a character can only have 1 race realm first (#23626) * chore: fix leftover license header (#23678) * fix(Scripts/HoL): Update Loken script (#23587) * fix(Scripts/DTK): Update King Dred script (#23572) * fix(DB/SAI): Bitter Departure quest credit (#23658) * chore(DB): import pending files Referenced commit(s):e595425578* fix(DB/Conditions): Ice Shard require Icy Imprisonment (#23661) * chore(DB): import pending files Referenced commit(s):8294652e77* fix(DB/Loot): add Scourge Curio drop to Lost Shandaral Spirit (#23686) * chore(DB): import pending files Referenced commit(s):b6ed4347fe* fix(DB/Gameobject): fix spell focus location for 'Will of the Titans' (#23683) * chore(DB): import pending files Referenced commit(s):388f18895d* fix(DB/Creature): update IOC Demolisher spells (#23685) * chore(DB): import pending files Referenced commit(s):cdfa50c990* fix(Scripts/Northrend): IOC boss cast ability Mortal Strike (#23684) * fix(Scripts/BoreanTundra): Fix Beryl Sorcerer engaging mobs (#23690) * fix(Core/Entities): Improve interactions between taxis and players regarding PvP flag. (#23681) * fix(DB/Creature): Peon Gakra should be an innkeeper (#23699) * chore(DB): import pending files Referenced commit(s):6abff4ac2b* fix(Scripts/SholazarBasin): Fix Song of Wind and Water double credit (#23707) * fix(DB/SAI): Reanimated Frost Wyrm engage after being hit by quest spell (#23697) * fix(DB/SAI): Timely respawn Nesingwary Trappers (#23703) * fix(DB/Creature): Fix Fjord Hawk Matriarch unit flags (#23696) * fix(DB/Conditions): Fix Fordragon Resolve target conditions (#23701) * chore(DB): import pending files Referenced commit(s):2942d63125* fix(DB/Script): Move Tailhorn Stag and Amberpine Woodsman behavior into SmartAI. (#23708) * fix(DB/Creature): Set Trigger flag on Steam Vent. (#23710) * chore(DB): import pending files Referenced commit(s):435ca302ef* fix(DB/SAI): To Stars' Rest! taxi flight (#23712) * chore(DB): import pending files Referenced commit(s):ab4d59ac9d* fix (DB/Creature): Set Surveyor Orlond flags. (#23714) * chore(DB): import pending files Referenced commit(s):e8ec77dca7* fix(DB/Loot): Fix Master Summoner Staff drop chance (#23717) * chore(DB): import pending files Referenced commit(s):182c055e6e* fix(Scripts/DTK): Fix Oh Novos! achievement (#23539) (#23718) * fix(Core/Spells): Remove King Mrgl-Mrgl costume on spell casting (#23713) * chore(DB): import pending files Referenced commit(s):8c963a11ce* fix(DB/Reputation): Utigarde Pinnacle normal reputation (#23719) * chore(DB): import pending files Referenced commit(s):88ed7d66d5* fix(Scripts/HoS): Clean up faction update hacks (#23720) * fix(DB/Reputation): Lower reputation according to rates handling (#23722) * fix(DB/Reputation): Oculus normal & UP correction (#23723) * chore(DB): import pending files Referenced commit(s):abc2cf3028* fix(Scripts/Oculus): Implement crossfaction support for drakes (#23704) * fix(DB/Quest): Correct prerequisite for Reclaimed Ration (#23736) Co-authored-by: blinkysc <blinkysc@users.noreply.github.com> * fix(DB/Quest): Correct prerequisite for Salvaging Life's Strength (#23734) Co-authored-by: blinkysc <blinkysc@users.noreply.github.com> * chore(DB): import pending files Referenced commit(s):afd8197588* fix(Core/Movement): Fix SummonMovementInform for summons (#23725) * refactor(Core/Movement): Fix Build (#23739) * fix(DB/SAI): Update Iron Rune Construct SAI to use DO_ACTION instead … (#23716) * chore(DB): import pending files Referenced commit(s):7cc39f78e2* fix(DB/SAI): Fix Flamebringer gossip interaction (#23740) * chore(DB): import pending files Referenced commit(s):9cb683cfcd* fix(DB/SAI): Nerub'ar member packs now attack together. (#23727) * chore(DB): import pending files Referenced commit(s):6f5a1b7ccc* fix(DB/SAI): Remove Harrison Johnes quest flag on escort accept (#23700) * chore(DB): import pending files Referenced commit(s):bacf15d356* Update crash issue template with log submission guidelines (#23754) * Merge * Updated OnPlayerChat method name to OnPlayerCanUseChat --------- Co-authored-by: sogladev <sogladev@gmail.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: 天鹭 <18535853+PkllonG@users.noreply.github.com> Co-authored-by: ModoX <moardox@gmail.com> Co-authored-by: Shauren <shauren.trinity@gmail.com> Co-authored-by: Grimdhex <237474256+Grimdhex@users.noreply.github.com> Co-authored-by: sudlud <sudlud@users.noreply.github.com> Co-authored-by: dr-j <dr-j@users.noreply.github.com> Co-authored-by: Killyana <morphone1@gmail.com> Co-authored-by: Undo <50205200+UndoUreche@users.noreply.github.com> Co-authored-by: Andrew <47818697+Nyeriah@users.noreply.github.com> Co-authored-by: killerwife <killerwife@gmail.com> Co-authored-by: Tereneckla <Tereneckla@pm.me> Co-authored-by: Rocco Silipo <108557877+Rorschach91@users.noreply.github.com> Co-authored-by: Ryan Turner <16946913+TheSCREWEDSoftware@users.noreply.github.com> Co-authored-by: blinkysc <37940565+blinkysc@users.noreply.github.com> Co-authored-by: Francesco Borzì <borzifrancesco@gmail.com> Co-authored-by: Benjamin Jackson <38561765+heyitsbench@users.noreply.github.com> Co-authored-by: Traesh <Traesh@users.noreply.github.com> Co-authored-by: blinkysc <blinkysc@users.noreply.github.com>
This commit is contained in:
kadeshar
Vendored
+174
@@ -0,0 +1,174 @@
|
||||
<!--
|
||||
Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
|
||||
|
||||
SPDX-License-Identifier: curl
|
||||
-->
|
||||
|
||||
# How to do code reviews for curl
|
||||
|
||||
Anyone and everyone is encouraged and welcome to review code submissions in
|
||||
curl. This is a guide on what to check for and how to perform a successful
|
||||
code review.
|
||||
|
||||
## All submissions should get reviewed
|
||||
|
||||
All pull requests and patches submitted to the project should be reviewed by
|
||||
at least one experienced curl maintainer before that code is accepted and
|
||||
merged.
|
||||
|
||||
## Let the tools and tests take the first rounds
|
||||
|
||||
On initial pull requests, let the tools and tests do their job first and then
|
||||
start out by helping the submitter understand the test failures and tool
|
||||
alerts.
|
||||
|
||||
## How to provide feedback to author
|
||||
|
||||
Be nice. Ask questions. Provide examples or suggestions of improvements.
|
||||
Assume the best intentions. Remember language barriers.
|
||||
|
||||
All first-time contributors can become regulars. Let's help them go there.
|
||||
|
||||
## Is this a change we want?
|
||||
|
||||
If this is not a change that seems to be aligned with the project's path
|
||||
forward and as such cannot be accepted, inform the author about this sooner
|
||||
rather than later. Do it gently and explain why and possibly what could be
|
||||
done to make it more acceptable.
|
||||
|
||||
## API/ABI stability or changed behavior
|
||||
|
||||
Changing the API and the ABI may be fine in a change but it needs to be done
|
||||
deliberately and carefully. If not, a reviewer must help the author to realize
|
||||
the mistake.
|
||||
|
||||
curl and libcurl are similarly strict on not modifying existing behavior. API
|
||||
and ABI stability is not enough, the behavior should also remain intact as far
|
||||
as possible.
|
||||
|
||||
## Code style
|
||||
|
||||
Most code style nits are detected by checksrc but not all. Only leave remarks
|
||||
on style deviation once checksrc does not find anymore.
|
||||
|
||||
Minor nits from fresh submitters can also be handled by the maintainer when
|
||||
merging, in case it seems like the submitter is not clear on what to do. We
|
||||
want to make the process fun and exciting for new contributors.
|
||||
|
||||
## Encourage consistency
|
||||
|
||||
Make sure new code is written in a similar style as existing code. Naming,
|
||||
logic, conditions, etc.
|
||||
|
||||
## Are pointers always non-NULL?
|
||||
|
||||
If a function or code rely on pointers being non-NULL, take an extra look if
|
||||
that seems to be a fair assessment.
|
||||
|
||||
## Asserts
|
||||
|
||||
Conditions that should never be false can be verified with `DEBUGASSERT()`
|
||||
calls to get caught in tests and debugging easier, while not having an impact
|
||||
on final or release builds.
|
||||
|
||||
## Memory allocation
|
||||
|
||||
Can the mallocs be avoided? Do not introduce mallocs in any hot paths. If
|
||||
there are (new) mallocs, can they be combined into fewer calls?
|
||||
|
||||
Are all allocations handled in error paths to avoid leaks and crashes?
|
||||
|
||||
## Thread-safety
|
||||
|
||||
We do not like static variables as they break thread-safety and prevent
|
||||
functions from being reentrant.
|
||||
|
||||
## Should features be `#ifdef`ed?
|
||||
|
||||
Features and functionality may not be present everywhere and should therefore
|
||||
be `#ifdef`ed. Additionally, some features should be possible to switch on/off
|
||||
in the build.
|
||||
|
||||
Write `#ifdef`s to be as little of a "maze" as possible.
|
||||
|
||||
## Does it look portable enough?
|
||||
|
||||
curl runs "everywhere". Does the code take a reasonable stance and enough
|
||||
precautions to be possible to build and run on most platforms?
|
||||
|
||||
Remember that we live by C89 restrictions.
|
||||
|
||||
## Tests and testability
|
||||
|
||||
New features should be added in conjunction with one or more test cases.
|
||||
Ideally, functions should also be written so that unit tests can be done to
|
||||
test individual functions.
|
||||
|
||||
## Documentation
|
||||
|
||||
New features or changes to existing functionality **must** be accompanied by
|
||||
updated documentation. Submitting that in a separate follow-up pull request is
|
||||
not OK. A code review must also verify that the submitted documentation update
|
||||
matches the code submission.
|
||||
|
||||
English is not everyone's first language, be mindful of this and help the
|
||||
submitter improve the text if it needs a rewrite to read better.
|
||||
|
||||
## Code should not be hard to understand
|
||||
|
||||
Source code should be written to maximize readability and be easy to
|
||||
understand.
|
||||
|
||||
## Functions should not be large
|
||||
|
||||
A single function should never be large as that makes it hard to follow and
|
||||
understand all the exit points and state changes. Some existing functions in
|
||||
curl certainly violate this ground rule but when reviewing new code we should
|
||||
propose splitting into smaller functions.
|
||||
|
||||
## Duplication is evil
|
||||
|
||||
Anything that looks like duplicated code is a red flag. Anything that seems to
|
||||
introduce code that we *should* already have or provide needs a closer check.
|
||||
|
||||
## Sensitive data
|
||||
|
||||
When credentials are involved, take an extra look at what happens with this
|
||||
data. Where it comes from and where it goes.
|
||||
|
||||
## Variable types differ
|
||||
|
||||
`size_t` is not a fixed size. `time_t` can be signed or unsigned and have
|
||||
different sizes. Relying on variable sizes is a red flag.
|
||||
|
||||
Also remember that endianness and >= 32-bit accesses to unaligned addresses
|
||||
are problematic areas.
|
||||
|
||||
## Integer overflows
|
||||
|
||||
Be careful about integer overflows. Some variable types can be either 32-bit
|
||||
or 64-bit. Integer overflows must be detected and acted on *before* they
|
||||
happen.
|
||||
|
||||
## Dangerous use of functions
|
||||
|
||||
Maybe use of `realloc()` should rather use the dynbuf functions?
|
||||
|
||||
Do not allow new code that grows buffers without using dynbuf.
|
||||
|
||||
Use of C functions that rely on a terminating zero must only be used on data
|
||||
that really do have a null-terminating zero.
|
||||
|
||||
## Dangerous "data styles"
|
||||
|
||||
Make extra precautions and verify that memory buffers that need a terminating
|
||||
zero always have exactly that. Buffers *without* a null-terminator must not be
|
||||
used as input to string functions.
|
||||
|
||||
# Commit messages
|
||||
|
||||
Tightly coupled with a code review is making sure that the commit message is
|
||||
good. It is the responsibility of the person who merges the code to make sure
|
||||
that the commit message follows our standard (detailed in the
|
||||
[CONTRIBUTE](CONTRIBUTE.md) document). This includes making sure the PR
|
||||
identifies related issues and giving credit to reporters and helpers.
|
||||
Reference in New Issue
Block a user