-
Notifications
You must be signed in to change notification settings - Fork 79
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
feat: validate new issues GHA #2190
Conversation
@jshackell-sfdc 3 markdown files for review here. The rendered templates can be viewed here: https://github.com/iowillhoit/gha-sandbox/issues/22 |
.github/ISSUE_TEMPLATE/bug_report.md
Outdated
|
||
### System Information | ||
<!-- Which shell/terminal are you using? (bash, zsh, powershell 7, cmd.exe, etc) --> | ||
<!-- Paste the **full** output of the `version --verbose --json` command below --> |
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.
suggestion: does doctor provide anything better that the version --verbose doesn't that we might want instead or in addition?
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.
Doctor does some version and plugin checks but it does not actually output any of it. I think we still want to suggest this for now so we get version, plugins, os, node version, etc
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.
some code comments and process questions.
required: true | ||
description: "Token taken from secrets env var" | ||
runs: | ||
using: "node16" |
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'd go higher since 16 is EOL in October.
I couldn't tell from the docs if there's a way to specify lts, but that would be best.
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's not supported yet 😑 actions/runner-images#7002 (comment)
I tried a work around there but it did not seem to work. Going to wait it out and hopefully they will add support
@@ -46,6 +46,21 @@ jobs: | |||
If you require immediate assistance, contact Salesforce Customer Support. | |||
- name: log action output | |||
run: echo ${{steps.run_action.outputs.message}} | |||
validate-new-issue: |
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.
if I get your auto-response [say I forgot to put my version stuf], and then I modify the issue to include that, will this rerun? I'm thinking no because it's triggered off of new labels.
We'd probably want it to "maintain" the more information required
label until the issue meets the criteria (it might not need to comment again if the previous comment still stands, but it could make sure the label stays on)?
Maybe it should be triggered by something else (create/edit of an issue ?)
owner, | ||
repo, | ||
issue_number, | ||
labels: ["more information needed"], |
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.
what removes the more information needed
label? Should it remove investigating
Would it be good if additional info from the user (provided it meets the criteria) would
- remove
more info
- add
investigating
back ?
I'm trying to think of the easiest way for the Trust project to be accurate more often
Ok @mshanemc , updated this to add and remove labels. Also added another workflow that will watch for updates to the PR body and comments. I think we have good coverage on scenarios that we will encounter. I will try to monitor this and make sure it is working as expected over the coming weeks. See the note about adding a Testing comments: https://github.com/iowillhoit/gha-sandbox/issues/32 Edit: |
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.
seems good, thanks for thinking through the label states.
// Temporary check to prevent this action from running on old issues | ||
// This will prevent noise on tickets already being investigated | ||
// This can be removed once the action has been running for a while | ||
const creationDate = new Date(issue.created_at); | ||
const cutoffDate = new Date("2023-06-11T00:00:00Z"); | ||
if (creationDate < cutoffDate) { | ||
console.log("Issue was created before 6/11/2023, skipping"); | ||
return; | ||
} |
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.
One more thing, I added a date check to prevent this from running on old issues. Here is an example of the check failing: https://github.com/iowillhoit/gha-sandbox/actions/runs/5245104855/jobs/9472071608#step:5:8
Adds a new Github action that validates new issues.
The Action will:
sfdx
orsf
are older than latest--verbose
version output was not includedmore information needed
labelHere is an example of all 3 cases: https://github.com/iowillhoit/gha-sandbox/issues/22
This PR also updates ISSUE_TEMPLATE
Testing Instructions:
@salesforce/cli@x.x.x
andsfdx-cli/x.x.x
pluginVersions
orPlugin Version:
(to support json and stdout)