Update issue manager with "feedback requested" behavior #361
Conversation
We weren't using an "answered" label, so I just replaced it. Triggers updated per documentation!
|
I specifically want to use this functionality for #356 |
|
|
||
| jobs: | ||
| issue-manager: | ||
| runs-on: ubuntu-latest | ||
| runs-on: "ubuntu-latest" |
There was a problem hiding this comment.
I feel strongly that quoting YAML strings is important. There's too many weird things that can happen if you don't. Just like bash :) Is there a pre-commit validator that can check for this? :)
There was a problem hiding this comment.
GitHub's own documentation doesn't quote simple YAML strings, except for ${{}} contexts, e.g.,
https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idruns-on
Other than a few special cases (number-y strings and special characters) I've not had any issues with unquoted strings in YAML. So, I feel it's entirely unnecessary and would prefer to match GitHub's documentation examples as much as possible.
There was a problem hiding this comment.
I don't think this is something with an objectively correct answer, but I also don't really consider GitHub an authority on YAML quality, so I'm not terribly swayed in my opinion by that :) One of the strange thing YAML does is convert the value no (the word "no" or the country code for Norway) to False. String rules in YAML are very complicated. I think "explicit is better than implicit", and "simple is better than complex" are my guiding principles in this preference.
If you feel strongly against adding the quotes, I don't think we're going to resolve that in the context of this PR :) I'm happy to restore this to a mix of quoted and unquoted stings as it was before (because I don't think we should enforce any standard when we don't have consensus) so we can get this merged!
There was a problem hiding this comment.
Following up on the earlier thought about validation, there is a yamllint rule that can enforce always-quoted strings (single or double), never enforce quotes, or permit quotes only when needed (for all the special cases like no, yes, true, false, #, [], {}, \d.*... probably more I'm not aware of). If/when we have consensus on this issue, I think it'd be great to record that decision as a linting rule.
https://yamllint.readthedocs.io/en/stable/rules.html#module-yamllint.rules.quoted_strings
There was a problem hiding this comment.
I don't think my preference is strong enough to warrant a standoff 🤣
I've just rarely hit this issue in practice, and almost always with trailing zeros on number-y things. So I see a molehill here, but if you've climbed a mountain and planted a flag, that's cool with me -- we'll go with that.
I do like the idea of making a decision and letting the linter-bot be the sheriff, though!
There was a problem hiding this comment.
I do like the idea of making a decision and letting the linter-bot be the sheriff, though!
Yes! Having software solve these problems is much better than dealing with competing preferences. There are lots of usually-not-a-big-deal things people can have strong opinions about, and for me quoting strings in YAML and Bash is one of them. I've been bitten too many times, and "just quote strings" keeps my cognitive load low :) But I don't want things that help me to cause harm to anyone else, so... yeah, let's make this a discussion about linters/formatters :) Opened #364
Co-authored-by: Joseph H Kennedy <[email protected]>
|
Looks to be working: https://github.com/nsidc/earthaccess/actions/runs/6965375261/job/18953956336 |
We weren't using an "answered" label, so I just replaced it.
Triggers updated per documentation!