Merge ~rodrigo-zaiden/ubuntu-cve-tracker:generate_usn_grepping into ubuntu-cve-tracker:master

[Rodrigo Figueiredo Zaiden]

while running the USN bash scripts, the grep that is supposed to find FIXMEs and placeholders is pointing to comment lines, like below example:

 bash new-usn.sh
 new-usn.sh:36:# title: used for Email Subject, Web title. XXX-EXPAND-TO-UPSTREAM-NAME-XXX
 new-usn.sh:57:# releases the same. XXX-CHECK-XXX
 new-usn.sh still contains placeholders / FIXMEs - ensure you fix these all then rerun the script

with this commit, I'm adding a regex to remove lines starting with "#" that should remove the check for comment lines.

[Alex Murray (alexmurray)]

My preference is to be more strict and check any instance of XXX since these comments still contain actionable items that should be addressed.

[Alex Murray (alexmurray)]

I would be interested to hear what other members of the team think though - so have just assigned ubuntu-security to review this as well.

[Spyros Seimenis (sespiros)]

I agree with Alex, I thought the purpose of adding that regex check was to force someone to look at the XXX in the commented lines (in addition to making sure that no placeholder XXX text has been forgotten in a description). Instead of ignoring those specific 3 cases why not:
- XXX-EXPAND-TO-UPSTREAM-NAME-XXX, automate that step instead of requiring a manual check
- XXX-CHECK-XXX not sure if that can be automated

or simply use 2 different identifiers, for example XXX for placeholder texts and TODO/MANUALCHECK/whatever, for those commented lines that simply require manual intervention.

[Amir Naseredini (sahnaseredini)]

I actually found it a bit too harsh to look for the placeholders inside the comments too, but that's a fair point from Alex as well.

[David Fernandez Gonzalez (litios)]

At first, I thought as Rodrigo but after reading the comments, I think this should stay as it is.

They are a good reminder of the different tasks we should do before publishing. We are already editing the USN so removing the placeholder won't take any time and is a way to "mark" that we indeed did them.

That said, I would even propose adding one more right before the binaries so it's a reminder of reducing them.

[Amir Naseredini (sahnaseredini)]

+1 to a reminder for reducing to minimum binaries :)

[Alex Murray (alexmurray)]

Proposed https://code.launchpad.net/~alexmurray/ubuntu-cve-tracker/+git/ubuntu-cve-tracker-1/+merge/438499 to add a XXX for reducing the binaries.

[Alex Murray (alexmurray)]

Formally disapproving this change from my side.

review: Disapprove

[Rodrigo Figueiredo Zaiden (rodrigo-zaiden)]

Thank you all for the inputs.
It does make sense, and I'm also +1 on including the part of reducing binaries.

I reviewed the kernel USNs and the XXX is also useful there, where we have the reminders to remove meta and signed kernels sources.

for now, I'll be formally rejecting the merge request.

Thanks!

Unmerged commits

c6953d8... by Rodrigo Figueiredo Zaiden on 2023-03-06

sis-generate-usn: add regex to remove comment lines in grep

 when grepping for placeholders, we shouldn't look the lines
 that are comments, that is, starting with '#'

Signed-off-by: Rodrigo Figueiredo Zaiden <email address hidden>

diff --git a/scripts/sis-generate-usn b/scripts/sis-generate-usn
index 8fa14c2..7a6b4cc 100755
--- a/scripts/sis-generate-usn
+++ b/scripts/sis-generate-usn
@@ -570,7 +570,7 @@ print('#!/bin/bash')
 print('set -e')
 print()
 print('# check for unfixed placeholders - DO NOT REMOVE THE FOLLOWING LINE')
-print('grep -Hn "X""X""X" "${0}" && echo "${0} still contains placeholders / FIXMEs - ensure you fix these all then rerun the script" && exit 1')
+print('grep -EHn "^[^#]*"X""X""X"" "${0}" && echo "${0} still contains placeholders / FIXMEs - ensure you fix these all then rerun the script" && exit 1')
 print()
 print('export PATH=$PATH:%s' % (config['usn_tool']))
 print('export USN=%s'%(usn))