Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: point to guidelines on failure in TAP output #95

Merged
merged 1 commit into from
Mar 1, 2022
Merged

feat: point to guidelines on failure in TAP output #95

merged 1 commit into from
Mar 1, 2022

Conversation

richardlau
Copy link
Member

If commit linting fails, include the URL to the commit message
guidelines in the TAP output.

Refs: nodejs/node#41697

This will output, e.g.

# 86c832519452f74a1c4a701ec086509847d2485f
ok 1 co-authored-by-is-trailer: no Co-authored-by metadata
ok 2 fixes-url: skipping fixes-url # SKIP
ok 3 line-after-title: blank line after title
ok 4 line-length: line-lengths are valid
not ok 5 subsystem: Invalid subsystem: "feat" (feat: point to guidelines on failure in TAP output)
  ---
    {
      found: 'feat: point to guidelines on failure in TAP output',
      compare: 'indexOf() !== -1',
      wanted: [
        'benchmark',      'build',    'bootstrap',   'cli',
        'deps',           'doc',      'errors',      'etw',
        'esm',            'gyp',      'inspector',   'lib',
        'loader',         'meta',     'msi',         'node',
        'node-api',       'perfctr',  'policy',      'src',
        'test',           'tools',    'typings',     'wasm',
        'win',            'assert',   'async_hooks', 'buffer',
        'child_process',  'cluster',  'console',     'constants',
        'crypto',         'debugger', 'dgram',       'dns',
        'domain',         'events',   'fs',          'http',
        'http2',          'https',    'inspector',   'module',
        'net',            'os',       'path',        'perf_hooks',
        'process',        'punycode', 'querystring', 'quic',
        'readline',       'repl',     'report',      'stream',
        'string_decoder', 'sys',      'timers',      'tls',
        'trace_events',   'tty',      'url',         'util',
        'v8',             'vm',       'wasi',        'worker',
        'zlib'
      ],
      at: { line: 0, column: 0, body: undefined }
    }
  ...
ok 6 title-format: Title is formatted correctly.
ok 7 title-length: Title is <= 50 columns.

0..7
# tests 7
# pass  5
# fail  1
# Please review the commit message guidelines:
# https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

When clicking on "Details" for a failed GitHub actions check the URL has ?check_suite_focus=true appended, e.g. https://github.com/nodejs/node/runs/4741156729?check_suite_focus=true, which automatically expands the failing part of the workflow so this additional output will be immediately above the Error: Process completed with exit code 123. in the expanded output.

@codecov
Copy link

codecov bot commented Feb 25, 2022 •
edited
Loading

Codecov Report

Merging #95 (72d2517) into main (69435db) will increase coverage by 18.22%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main      #95       +/-   ##
===========================================
+ Coverage   75.65%   93.88%   +18.22%     
===========================================
  Files          19       19               
  Lines         456      458        +2     
===========================================
+ Hits          345      430       +85     
+ Misses        111       28       -83
Impacted Files Coverage Δ
lib/tap.js 93.22% <100.00%> (+87.95%) ⬆️
bin/cmd.js 90.69% <0.00%> (+23.25%) ⬆️
lib/format-tap.js 66.66% <0.00%> (+61.90%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69435db...72d2517. Read the comment docs.

@richardlau richardlau mentioned this pull request Feb 25, 2022
Closed
@richardlau
feat: point to guidelines on failure in TAP output
72d2517 
If commit linting fails, include the URL to the commit message
guidelines in the TAP output.
@richardlau
Copy link
Member Author

I've added a test to cover the new lines.

@richardlau richardlau merged commit 0e137d0 into nodejs:main Mar 1, 2022
@richardlau richardlau deleted the with-contrib-url branch March 1, 2022 12:14
@richardlau richardlau mentioned this pull request Mar 1, 2022
Merged
nodejs-github-bot pushed a commit to nodejs/node that referenced this pull request Mar 3, 2022
@richardlau
build: drop shortened URL from lint-commit-message
2d6aea7 
The shortened link to the commit message guidelines no longer works
after they were moved from `docs/guides` to `docs/contributing`. Now
that `core-validate-commit` outputs an error message pointing to the
full URL of the commit message guidelines on failure we no longer need
to include the URL (shortened or otherwise) in the workflow title.

PR-URL: #42168
Refs: nodejs/core-validate-commit#95
Refs: #41697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
sxa pushed a commit to sxa/node that referenced this pull request Mar 7, 2022
@richardlau @sxa
build: drop shortened URL from lint-commit-message
0bbb447 
The shortened link to the commit message guidelines no longer works
after they were moved from `docs/guides` to `docs/contributing`. Now
that `core-validate-commit` outputs an error message pointing to the
full URL of the commit message guidelines on failure we no longer need
to include the URL (shortened or otherwise) in the workflow title.

PR-URL: nodejs#42168
Refs: nodejs/core-validate-commit#95
Refs: nodejs#41697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams pushed a commit to danielleadams/node that referenced this pull request Apr 21, 2022
@richardlau @danielleadams
build: drop shortened URL from lint-commit-message
3391f2d 
The shortened link to the commit message guidelines no longer works
after they were moved from `docs/guides` to `docs/contributing`. Now
that `core-validate-commit` outputs an error message pointing to the
full URL of the commit message guidelines on failure we no longer need
to include the URL (shortened or otherwise) in the workflow title.

PR-URL: nodejs#42168
Refs: nodejs/core-validate-commit#95
Refs: nodejs#41697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams pushed a commit to nodejs/node that referenced this pull request Apr 24, 2022
@richardlau @danielleadams
build: drop shortened URL from lint-commit-message
2926f0b 
The shortened link to the commit message guidelines no longer works
after they were moved from `docs/guides` to `docs/contributing`. Now
that `core-validate-commit` outputs an error message pointing to the
full URL of the commit message guidelines on failure we no longer need
to include the URL (shortened or otherwise) in the workflow title.

PR-URL: #42168
Refs: nodejs/core-validate-commit#95
Refs: #41697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams pushed a commit to nodejs/node that referenced this pull request Apr 24, 2022
@richardlau @danielleadams
build: drop shortened URL from lint-commit-message
744e84e 
The shortened link to the commit message guidelines no longer works
after they were moved from `docs/guides` to `docs/contributing`. Now
that `core-validate-commit` outputs an error message pointing to the
full URL of the commit message guidelines on failure we no longer need
to include the URL (shortened or otherwise) in the workflow title.

PR-URL: #42168
Refs: nodejs/core-validate-commit#95
Refs: #41697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams pushed a commit to nodejs/node that referenced this pull request Apr 24, 2022
@richardlau @danielleadams
build: drop shortened URL from lint-commit-message
ba56e41 
The shortened link to the commit message guidelines no longer works
after they were moved from `docs/guides` to `docs/contributing`. Now
that `core-validate-commit` outputs an error message pointing to the
full URL of the commit message guidelines on failure we no longer need
to include the URL (shortened or otherwise) in the workflow title.

PR-URL: #42168
Refs: nodejs/core-validate-commit#95
Refs: #41697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
@richardlau @xtx1130
build: drop shortened URL from lint-commit-message
4d981fc 
The shortened link to the commit message guidelines no longer works
after they were moved from `docs/guides` to `docs/contributing`. Now
that `core-validate-commit` outputs an error message pointing to the
full URL of the commit message guidelines on failure we no longer need
to include the URL (shortened or otherwise) in the workflow title.

PR-URL: nodejs#42168
Refs: nodejs/core-validate-commit#95
Refs: nodejs#41697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evanlucas evanlucas evanlucas approved these changes

@targos targos targos approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@richardlau @evanlucas @targos @RaisinTen