-
Notifications
You must be signed in to change notification settings - Fork 482
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
Notify the release manager & slack integration improvements #4650
Conversation
- use action with version tag - import secrets from the vault
@misiekhardcore another good one for you to review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one thing I've noticed not related to functionality - is there any naming convention for workflow job names? cause I see some names with underscores, some with dashes and some in camel case
Looks like there's no convention at time. How would you solve it? Is there any convention endorsed by GH? |
I'm a fan of consistency, and would solve it using human readable labels, i.e. "Notify project channel" over "notifcyProjectChannel" or the like. GitHub allows actions to be human readable, and I'd use that. Our conventions for workflows are that they are |
For the human-readable label, we can use |
Proposal:
|
Implemented in a4b6c14 |
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
I personally always try to add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the unification, let's ensure we communicate with the team and/or check bpmn-js / diagram-js if we have clashes there.
We want to ensure that, as always, the "standard" is clearly visible on the happy path.
Merging this so we can build on top, i.e. with #4655. |
maybe its a good candidate for a spring cleaning issue to standardize the naming in workflows in other repositories? |
Yes, I'll have a quick look into diagram-js and bpmn-js, just so we have the changes in there. |
Follow-up for core repositories:
I also propose to unifiy our CI workflow names, cf. #4657. Looking forward to your @barmac and @misiekhardcore thoughts. |
I reviewed all of them and left a comment on the names changes |
Unfortunately, the automation I built in this PR did not work correctly today: https://github.com/camunda/camunda-modeler/actions/runs/11813602874/job/32910986873 |
It looks that the same output was also not handled correctly in another flow: https://github.com/camunda/camunda-modeler/actions/runs/11254745468/job/31292835752 I checked the action logs, and the
https://github.com/camunda/camunda-modeler/actions/runs/11813602874/job/32910976873#step:3:2 |
Before the change, the actions can fail silently. Cf. camunda/camunda-modeler#4650 (comment)
Before the change, the actions can fail silently. Cf. camunda/camunda-modeler#4650 (comment)
Proposed Changes
As discussed today in the morning, we want the release manager update to be explicitly notified to the team. This allows to react early in case the new release manager is not available. The PR is aiming to achieve that.
In the boy scout rule spirit, I also unified the slack integration in this repo to use the same version tag of the action, and consistently import the secrets from the vault.
Checklist
To ensure you provided everything we need to look at your PR:
@bpmn-io/sr
toolCloses {LINK_TO_ISSUE}
orRelated to {LINK_TO_ISSUE}