Skip to content

.github: bump slack notify to v3; enhance linter notifications#22196

Draft
jmank88 wants to merge 1 commit intodevelopfrom
notify-slack-v3
Draft

.github: bump slack notify to v3; enhance linter notifications#22196
jmank88 wants to merge 1 commit intodevelopfrom
notify-slack-v3

Conversation

@jmank88
Copy link
Copy Markdown
Contributor

@jmank88 jmank88 commented Apr 27, 2026

No description provided.

@jmank88 jmank88 requested review from chainchad and erikburt April 27, 2026 17:50
Comment on lines +105 to +108
xmllint --xpath "//error/@severity" checkstyle-result.xml | \
tr ' ' '\n' | \
sed 's/severity="//;s/"//' | \
sort | uniq -c | sort -nr | \
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was AI generated - maybe there is a cleaner way with pure xmllint?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 27, 2026

✅ No conflicts with other open PRs targeting develop

payload: |
channel: ${{ secrets.SLACK_TEAM_CORE_CHANNEL_ID}}
text: "golangci-lint failed (${{ matrix.modules }}): <${{ format('https://github.com/{0}/actions/runs/{1}', github.repository, github.run_id) }}|Run> - <${{ steps.golang-lint.outputs.golang-report-artifact-url }}|Report>"
text: "golangci-lint failed (${{ matrix.modules }}): <${{ format('https://github.com/{0}/actions/runs/{1}', github.repository, github.run_id) }}|Run> - <${{ steps.golang-lint.outputs.golang-report-artifact-url }}|Report>\n${{ steps.golang-lint.outputs.golang-report-file-count }} files with ${{ steps.golang-lint.outputs.golang-report-issue-count }} issues.\nBy severity: $${{ steps.golang-lint.outputs.golang-report-per-severity-count }}\nBy linter: ${{ steps.golang-lint.outputs.golang-report-per-source-count }}"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is probably a more elegant way to format this message via structured json or whatever.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What we've done before is to build the message in its own step and then include the body as a payload like: https://github.com/smartcontractkit/.github/blob/main/actions/slack-notify-git-ref/action.yml#L152

@trunk-io
Copy link
Copy Markdown

trunk-io Bot commented Apr 27, 2026

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@cl-sonarqube-production
Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

Comment on lines +95 to +96
cat ./${{ steps.set-working-directory.outputs.golangci-lint-working-directory }}golangci-lint-report.xml
echo "file-count=$( xmllint --xpath "count(//file)" checkstyle-result.xml )" >> $GITHUB_OUTPUT
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
cat ./${{ steps.set-working-directory.outputs.golangci-lint-working-directory }}golangci-lint-report.xml
echo "file-count=$( xmllint --xpath "count(//file)" checkstyle-result.xml )" >> $GITHUB_OUTPUT
cat "./${GOLANG_CI_LINT_DIR}golangci-lint-report.xml"
echo "file-count=$( xmllint --xpath "count(//file)" checkstyle-result.xml )" | tee -a "$GITHUB_OUTPUT"

Would always avoid the template inclusion in bash. Instead you can add it to an env within the step like:

env:
  GOLANG_CI_LINT_DIR: ${{ steps.set-working-directory.outputs.golangci-lint-working-directory }}

This mitigates template injection. Even for values that we control, it's probably worth doing by default.

Also, this is a nit but would suggest redirecting outputs to tee so we can see them in the GHA logs.

Comment on lines +95 to +96
cat ./${{ steps.set-working-directory.outputs.golangci-lint-working-directory }}golangci-lint-report.xml
echo "file-count=$( xmllint --xpath "count(//file)" checkstyle-result.xml )" >> $GITHUB_OUTPUT
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is xmllint installed on these runners by default?

I don't see it here: https://github.com/search?q=repo%3Aactions%2Frunner-images%20xmllint&type=code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh did gemini lie to me? 🤦

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we just output a different format, like JSON? I think we can do multiple, right? So it wouldn't have to be disruptive

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like using JSON output since we have the native tools on the runner 👍

payload: |
channel: ${{ secrets.SLACK_TEAM_CORE_CHANNEL_ID}}
text: "golangci-lint failed (${{ matrix.modules }}): <${{ format('https://github.com/{0}/actions/runs/{1}', github.repository, github.run_id) }}|Run> - <${{ steps.golang-lint.outputs.golang-report-artifact-url }}|Report>"
text: "golangci-lint failed (${{ matrix.modules }}): <${{ format('https://github.com/{0}/actions/runs/{1}', github.repository, github.run_id) }}|Run> - <${{ steps.golang-lint.outputs.golang-report-artifact-url }}|Report>\n${{ steps.golang-lint.outputs.golang-report-file-count }} files with ${{ steps.golang-lint.outputs.golang-report-issue-count }} issues.\nBy severity: $${{ steps.golang-lint.outputs.golang-report-per-severity-count }}\nBy linter: ${{ steps.golang-lint.outputs.golang-report-per-source-count }}"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What we've done before is to build the message in its own step and then include the body as a payload like: https://github.com/smartcontractkit/.github/blob/main/actions/slack-notify-git-ref/action.yml#L152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants