1. 
   LuizWT commented at 7:38 pm on September 18, 2024: none

   PR:

   This pull request optimizes the handling of trusted keys by converting lists to sets. This change improves performance for membership checks, making them faster.

   Tests

   No new tests were added, as this is a refactor that does not change the business logic.

2. Optimize: convert trusted keys list to a set for better performance d3e179c7f3
3. 
   DrahtBot commented at 7:38 pm on September 18, 2024: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Approach NACK	l0rinc

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. 
   laanwj commented at 7:58 pm on September 18, 2024: member

   Have you benchmarked the performance difference?
5. in contrib/verify-commits/verify-commits.py:86 in d3e179c7f3

   82@@ -83,17 +83,17 @@ def main():
   83     dirname = os.path.dirname(os.path.abspath(__file__))
   84     print("Using verify-commits data from " + dirname)
   85     with open(dirname + "/trusted-git-root", "r", encoding="utf8") as f:
   86-        verified_root = f.read().splitlines()[0]
   87+        verified_root = set(f.read().splitlines())[0]
   

   ---

   ---

   

   laanwj commented at 7:59 pm on September 18, 2024:

   Have you even tested this? You can’t subscript index into a set, and it makes no sense.

   ---

   

   SanAndreyas commented at 9:15 pm on September 18, 2024:

   This is not even an equivalent expression. When you convert a list to a set, the order of the lines is lost. Therefore, accessing the first element with [0] could give you any of the lines randomly, not necessarily the first line from the original file. In this case you needed index 0 ( root ).

   ---

   

   LuizWT commented at 3:56 am on September 19, 2024:

   This is not even an equivalent expression. When you convert a list to a set, the order of the lines is lost. Therefore, accessing the first element with [0] could give you any of the lines randomly, not necessarily the first line from the original file. In this case you needed index 0 ( root ).

   You’re right. It makes no sense to use a set in this context. Thanks for pointing this out

6. 
   l0rinc commented at 9:56 pm on September 18, 2024: contributor

   Approach NACK - no explanation, no benchmarks, no example for how to test this, etc
7. LuizWT closed this on Sep 19, 2024

   ---

Contributors
LuizWT DrahtBot laanwj SanAndreyas l0rinc