Skip to content

Clarify the missing baseline lines of code warning message - #475

Merged
aeisenberg merged 2 commits into
mainfrom
aeisenberg/warning-message
May 5, 2021
Merged

Clarify the missing baseline lines of code warning message#475
aeisenberg merged 2 commits into
mainfrom
aeisenberg/warning-message

Conversation

@aeisenberg

@aeisenbergaeisenberg commented May 5, 2021

Copy link
Copy Markdown
Contributor

See github/codeql#5838

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

henrymercer
henrymercer reviewed May 5, 2021
View reviewed changes
Comment threadsrc/count-loc.ts Outdated
Comment threadsrc/count-loc.ts Outdated
Comment threadsrc/count-loc.ts Outdated
@aeisenberg
aeisenbergforce-pushed the aeisenberg/warning-message branch 2 times, most recently from 18e4667 to 9228317CompareMay 5, 2021 15:46
henrymercer
henrymercer approved these changes May 5, 2021
View reviewed changes

@henrymercerhenrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple more suggestions on the message

Comment threadsrc/count-loc.ts Outdated
Comment threadsrc/count-loc.ts Outdated
Comment threadsrc/count-loc.ts Outdated
Comment threadsrc/count-loc.ts Outdated
@aeisenberg

aeisenberg commented May 5, 2021

Copy link
Copy Markdown
ContributorAuthor

All good suggestions.

@aeisenberg
aeisenbergforce-pushed the aeisenberg/warning-message branch from 9228317 to faa2564CompareMay 5, 2021 15:55
@adityasharad

adityasharad commented May 5, 2021

Copy link
Copy Markdown
Contributor

Do you want to keep this at the warning level (which makes it visible in the Actions summary page), or drop it down to info/debug so it's only present in the logs view?

@aeisenberg

aeisenberg commented May 5, 2021

Copy link
Copy Markdown
ContributorAuthor

Hmmm...didn't realize that this would be raised to being visible in the summary page. This is a good point. I'll drop down its verbosity.

@aeisenberg

aeisenberg commented May 5, 2021

Copy link
Copy Markdown
ContributorAuthor

A debug message won't appear at all unless the debug flag is set to true. I'd be leaning towards using an info message.

@aeisenberg
aeisenbergforce-pushed the aeisenberg/warning-message branch from faa2564 to e04c62bCompareMay 5, 2021 16:30
@aeisenberg
aeisenberg merged commit 5321045 into mainMay 5, 2021
@aeisenberg
aeisenberg deleted the aeisenberg/warning-message branch May 5, 2021 18:20
@github-actionsgithub-actionsBot mentioned this pull request May 10, 2021
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityasharadadityasharadadityasharad approved these changes
@henrymercerhenrymercerhenrymercer approved these changes

Assignees

@henrymercerhenrymercer

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aeisenberg@adityasharad@henrymercer