Fix: broken link to refactoring requirements#372
Conversation
The end of Line 31 needs an edit: |
|
Fixed omitted word and also added QA's static address code note requirements - these requirements ONLY apply to refactored sets, they are NOT requirements for active devs creating or maintaining their own work. QA is discussing pointer and array standards and will update this doc further once they are adopted. |
|
|
||
| Static addresses shall be formatted as follows: | ||
|
|
||
| - Bracketed size at the beginning of every note, may include BE or BCD as appropriate [16-bit BE], [16-bit BCD], [16-bit BE BCD] |
There was a problem hiding this comment.
I will likely never refactor a set under these rules if you make me put it at the very beginning.
There was a problem hiding this comment.
This should not be quite this strict. I think that's a bad move.
There was a problem hiding this comment.
"Bracketed, on the first line of the note, at beginning or end" would much better match the current modern code note ecosystem (but better worded), IMO.
There was a problem hiding this comment.
QA discussed this and decided that a single format code note standard for refactoring is preferred over individual stylistic preferences. Many of us expressed difficulty parsing notes with sizes in unpredictable locations due to uncertain lengths of preceding descriptions. With prepended, bracketed sizes, we felt that it is very easy to navigate notes with an extremely small variance in indentation prior to a description - a character or two in the vast majority of cases.
One of the primary goals of the refactoring project is to transform old, haphazard sets into very consistent packages that future maintainers can easily work through without any presentation variation. We want to minimize any personal style preference and have these sets effectively archived in perpetuity as standardized as possible. We felt that the size consistently up front was the overall best long term solution.
That said, as someone whose resolved hundreds upon hundreds of inactive dev tickets and has put in a ton of maintenance work, I consider your perspective on this especially valuable. I'll bring that up to the team and discuss it again. My personal priority is more standardization than my preference about whether size goes up front or at the back.
There was a problem hiding this comment.
I personally prefer size up front, but I think a consistent and reasonable style should be required rather than a specific one. A set where some notes have size at beginning and some have size at end is difficult to parse, but having all the notes one way or the other doesn't make a difference.
The docs says "A refactored set should be able to pass a code review without needing corrections.", but I wouldn't require a jr to rewrite all their notes to have the size at the beginning - I would require they be consistent with a format that most devs can understand.
Bitflags not being bracketed is another that would dissuade me from refactoring, I find using [8-bit] is less clear than [Bitflags] or [Bitfield]. Its like marking a float as [32-bit] and saying that the decimal values make it clear its actually a float. And I'd argue that bitflag notes should never be more than 8-bit anyway.
What I'm afraid you'll see are devs fixing sets to their liking and not informing QA, so the set stays in the refactor hub and its more difficult to find sets that actually need refactoring. Its already a tough sell when I have to do extra work (fixing code notes, making save states) beyond just fixing the set for players and preventing further tickets.
There was a problem hiding this comment.
I'm going to close this PR. Someone else can run with this if they want.
4 participants
Submitted the initial PR with a broken link - now fixed.