Changes between Version 2 and Version 3 of Ticket #31949, comment 27


Ignore:
Timestamp:
Jan 5, 2023, 3:04:34 AM (2 years ago)
Author:
Carlton Gibson

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #31949, comment 27

    v2 v3  
    77As I see it, there are two groups of issue with the current approach:
    88
    9 * The bulk edit means the code is quite a few time not sensitive enough to the individual decorator. Looking at them, ''couldn't we write this one that way, or that one this way?'', is the thought. (I left some more specific comments on the PR itself.) If we address them one-by-one (or in small related groups, likely) we can write the best code for each case. There are repeated patterns and opportunities for code-reuse, but I think extracting them as we go is going to work better than trying to determine them in advance.
     9* The bulk edit means the code is quite a few times not sensitive enough to the individual decorator. Looking at them, ''couldn't we write this one that way, or that one this way?'', is the thought. (I left some more specific comments on the PR itself.) If we address them one-by-one (or in small related groups, likely) we can write the best code for each case. There are repeated patterns and opportunities for code-reuse, but I think extracting them as we go is going to work better than trying to determine them in advance.
    1010
    1111* Related, the tests for each decorator need to live with the existing related tests, not be centralised in one place. Five years hence, when we come to work on the login_required logic, say, we need to be able to open the relevants tests and see them all together. Having to track down a separate set of tests in another part of the test suite entirely would be a maintenance headache. If we address the decorators individually this tendency won't be there.
     
    1414
    1515
    16 I think the general idea is correct thought, and I don't see anything to stop
     16I think the general idea is correct though, and I don't see anything to stop
    1717us being able to process smaller `Refs #31949 -- ...` PRs relatively quickly.
    1818
Back to Top