-
Notifications
You must be signed in to change notification settings - Fork 53
CHANGE: @W-18394257@ Update DevOps Post-GA #1787
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
Conversation
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.
Note that the 1 through 5 doesn't show up to the user on the https://github.com/forcedotcom/sfdx-scanner/issues/new/choose page - they are just there to dictate ordering. But with that said - they do show up in the urls. Note that our documentation page over at https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/get-started.html#5-report-bugs-feedback-and-request-new-features links to https://github.com/forcedotcom/sfdx-scanner/issues/new?template=5-feature_request.yml explicitly. So if we wanted to move this, we'd need to first have the doc updated.
For now, just delete - but please don't rename anything.
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.
Thank you for recalling that! I will update.
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.
Is there a reason not to name this 1-general_feedback.yml?
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.
I know, 6 felt so weird! Do you think it works for that order? I kind of wanted general feedback to be last, but I don't have a strong preference.
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.
"General feedback" being last makes total sense, but if we're deleting 1-5, then we can just promote this to 1, right?
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.
Unfortunately we have to keep 5 as it is since we have a hard-coded link in our docs to it, so we can be 0,1,5 or 0,5,6 (for today, at least!)
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.
Fair enough.
| secrets: inherit | ||
| with: | ||
| github-token: ${{ secrets.SVC_CLI_BOT_GITHUB_TOKEN }} | ||
| # TODO: remove inputs after April Release; will default to minor |
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.
I read this comment as referring to the inputs property on the script object. If you remove that property, then the default value for release-type will just be minor, which is what we want.
Are you sure we're able to remove this workflow dispatch and just directly call the workflow as a step?
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.
Let me make a fork to double check this one 👍
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.
Good callout on this - I did need to make changes over with the fork to make this work! Here's a run that calls create-release-branch successfully: https://github.com/randi274/sfdx-scanner/actions/runs/14800941925
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.
It doesn't look like that commit has any changes. Is that expected?
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.
It was a lot of small commits over there, but this is the one that should work here: 91dd103
And then the code over there should be correct on dev now
| workflow_id: 'production-heartbeat.yml', | ||
| ref: 'dev-4' | ||
| }); | ||
| smoke-test: |
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.
This looks incorrect to me. I'm not seeing where the logic that would call out to PagerDuty is?
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.
Ah, good point! I don't think I connected that the purpose of that was to callout to PagerDuty. Is that what the createWorkflowDispatch is doing here?
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.
@jfeingold35 here's the successful run to the heartbeat! Thanks for walking through that together. There might be some additional improvements to be made here too (possible follow-up WI).
https://github.com/randi274/sfdx-scanner/actions/runs/14801505744/job/41561089789
I did notice in the output of the job that we don't have python installed either so I made a follow-up WI for that: https://github.com/randi274/sfdx-scanner/actions/runs/14801505744/artifacts/3052802686
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.
Note: Add a new issue config to remove the blank issue that appears. https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/configuring-issue-templates-for-your-repository
0cc89aa to
7e75a56
Compare
| labels: [] | ||
| body: | ||
| - type: dropdown | ||
| - type: checkboxes |
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.
@stephen-carter-at-sf I was able to get validation to work on checkboxes, and thought it looked a little cleaner:
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.
Great!
| @@ -0,0 +1,123 @@ | |||
| name: Report a Bug with a scanner command (v4) | |||
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.
I was thinking this could be the new catch-all for scanner bugs
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.
Nice
| @@ -0,0 +1 @@ | |||
| blank_issues_enabled: false No newline at end of file | |||
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.
This will prevent the blank issues option from appearing.
Additionally, if we want other repos to point elsewhere for issue tracking, we could do that using this config file. https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/configuring-issue-templates-for-your-repository
| @@ -0,0 +1,147 @@ | |||
| name: heartbeat-tests | |||
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.
@stephen-carter-at-sf I removed production-heartbeat, and now this is the singular heartbeat tests workflow. Does the name work for you, or any changes requested?
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.
I"m fine with name.
| java-version: '11' | ||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.10' |
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.
I set the python version to match what our docs require.
| RUN_LINK: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }} | ||
| run: | | ||
| # GHA env-vars don't have robust conditional logic, so we'll use this if-else branch to define some bash env-vars. | ||
| ALERT_SEV="info" |
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.
Here is the change to update our Alert Severity to "info".
| labels: [] | ||
| body: | ||
| - type: dropdown | ||
| - type: checkboxes |
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.
Great!
| @@ -0,0 +1,123 @@ | |||
| name: Report a Bug with a scanner command (v4) | |||
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.
Nice
| attributes: | ||
| label: Python Version | ||
| description: | | ||
| What do you get from the command "python --version"? | ||
| placeholder: | | ||
| Example: Python 3.11.8 |
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.
We don't need python for scanner command... so we don't need this
| @@ -0,0 +1,147 @@ | |||
| name: heartbeat-tests | |||
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.
I"m fine with name.

Notes: