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.

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

GHA: Notify Mattermost #5274

Draft
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

malmstein
Copy link
Contributor

Task/Issue URL:

Description

Steps to test this PR

Feature 1

  • [ ]
  • [ ]

UI changes

Before After
!(Upload before screenshot) (Upload after screenshot)

Copy link
Contributor Author

malmstein commented Nov 14, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

@malmstein malmstein force-pushed the feature/david/11-14-gha_notify_mattermost branch from 980c67a to 24a1de6 Compare November 22, 2024 16:44
Comment on lines +39 to +53
run: |
./send_chat_message.sh '${{env.emoji_info}}'" Release task created https://github.com/duckduckgo/Android/actions/runs/${{ github.run_id }}"

- name: Send Release tag created
run: |
./send_chat_message.sh '${{env.emoji_info}}'" Release task created https://github.com/duckduckgo/Android/actions/runs/${{ github.run_id }}"

- name: Send Release task created message
run: |
echo -n '::notice title=Inputs::' && echo '${{ toJson(inputs) }}' | sed -z 's_\n_%0A_g'; echo
message="Release ${{ env.APP_VERSION }} task created"
message+="See https://github.com/duckduckgo/Android/actions/runs/${{ github.run_id }}"
./send_chat_message.sh '${{env.emoji_info}}'" ${message}"
./send_chat_message.sh '${{env.emoji_info}}'" Release task created https://github.com/duckduckgo/Android/actions/runs/${{ github.run_id }}"
Copy link

Choose a reason for hiding this comment

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

There appear to be several duplicate message sends in this workflow. Specifically:

  1. The Send Release tag created step sends the exact same message as the previous step
  2. The final step sends the "Release task created" message twice

Consider consolidating these into single message sends to avoid duplicate notifications in the chat channel. This will keep the notifications clean and meaningful for the team.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

token: ${{ secrets.GT_DAXMOBILE }}
fetch-depth: 0
ref: feature/david/11-14-gha_notify_mattermost
Copy link

Choose a reason for hiding this comment

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

The ref parameter is hardcoded to a specific feature branch, which will prevent this workflow from running on other branches. Consider removing this line to allow the workflow to execute on the current branch where it's triggered.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@malmstein malmstein force-pushed the feature/david/11-14-gha_notify_mattermost branch from 24a1de6 to 46857dc Compare December 9, 2024 20:54
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.

1 participant