get-metadata: missing 'LGTM' in comments

[vsemozhetbyt]

PR: nodejs/node#18630

Command: winpty get-metadata.cmd 18630 --check-comments

Output:

------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/18630
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
--------------------------------------------------------------------------------

Missing: nodejs/node#18630 (comment)

[priyank-p]

@vsemozhetbyt just want to confirm did the get-metadata output had something that said maclover7 approved via LGTM in comment or something along those lines? since this is all we do currently and it's not included in generated metadata.

[vsemozhetbyt]

@cPhost

Full output:

√  Done loading data for nodejs/node/pull/18630
----------------------------------- PR info ------------------------------------
Title      doc: fix link in https.md (#18630)
Author     Vse Mozhet Byt <[email protected]> (@vsemozhetbyt)
Branch     vsemozhetbyt:doc-fix-link-2018-02-07 -> nodejs:master
Labels     doc, fast-track, https, ready
Commits    1
 - doc: fix link in https.md
Committers 1
 - Vse Mozhet Byt <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/18630
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
--------------------------------------------------------------------------------
√  Requested Changes: 0
√  Approvals: 2, 1 from TSC (cjihrig)
i  Last Full CI on 2018-02-07T21:39:06Z: https://ci.nodejs.org/job/node-test-pull-request-lite/159/
i  This PR is being fast-tracked
‼  This PR is closed

[priyank-p]

Ok thanks, this is a bug.

[joyeecheung]

I think we have already discussed about this. This is by design - only comments that contain nothing but LGTM will get a warning, other types of comments don't get anything at all, because there will always be "something LGTM but I am not the expert in other things", or "I cannot give a LGTM for this" or "I would want @ someone 's LGTM for this " or other types of thing that don't make them legitimate sign-offs. In the end we will have to resort to natural language processing.

[joyeecheung]

BTW in those cases, something like #140 would be more suitable because without NLP code, humans are better at deciding if a comment with "LGTM" is legitimate sign-off

[priyank-p]

I think we have already discussed about this. This is by design - only comments that contain nothing but LGTM will get a warning

Oh, okay it does make sense now, so this is not a bug. I glanced at the PR again quickly to get some idea all i got was we don't include in metadata which i already knew, and something that said <name>(<username>) approved via LGTM in comment) 😅.

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

[codebytere]

Gonna close this one since it's pretty old and i don't think this is something there's wide interest in doing but please reopen if that's not the case!