-
Notifications
You must be signed in to change notification settings - Fork 98
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
Initial draft for triaging #901
base: main
Are you sure you want to change the base?
Conversation
I will automatically update this comment whenever this PR is modified
|
Nice work! Will look more closely this week. Looks like there's a tradeoff to be considered here to fix linting with |
I'm wondering if some updates may be needed based on the recently closed #760 ? If it would be helpful for me to review and update accordingly, I'd be happy to help. |
@asteiker that would be super helpful!! |
@asteiker OK to assign this PR to you? |
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.
🙌 Amazing work! Sorry about the number of suggestions here -- we re-organized labels in #760 to be more self-describing, and I think that helps make this document more clear!
@asteiker can you also take a look? |
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
@mfisher87 Hey Matt, what about PR triaging workflows? Wouldn't they also be included in the triaging doc? |
I'm not sure exactly what you mean here; are you thinking about PR reviews? |
Yeah PR reviews workflow(I confused it with PR triaging), there is a flowchart for that as well in the repo you linked, should we add that as well to our triaging guide? |
I think it's a great idea! But I would also like to keep this PR focused and get it merged without too much delay. What do you think about dealing with that in another PR? |
@Sherwin-14 I'd like to wait to review until the ReadTheDocs build is working again. Do you feel comfortable fixing it? Details here: #901 (comment) |
Sounds good to me :). I also have added the mermaid dependency. |
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.
In a rush, just leaving comments :)
- **needs: decision**: We're struggling to decide what to do and the decision committee needs to help. | ||
- **needs: feedback**: Use this label for issues where feedback is requested from the team or our community. | ||
- **needs: help**: Use this label for issues where additional help or contributions are 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.
I'd like to add back the "good first issue" label, perhaps alone in a "Special labels" section? That is one that triagers should be especially aware of!
start == YES ==> dupe{Is duplicate?} | ||
dupe == YES ==> close2[Close and point to duplicate] | ||
dupe == NO ==> repro{Has proper reproduction?} | ||
repro == NO ==> close3[Label: 'needs reproduction' bot will auto close if no update has been made in 3 days] |
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.
bot will auto close if no update has been made in 3 days
I think this isn't true. We have one automation for autoclosing issues, but I believe it's broken: https://github.com/nsidc/earthaccess/blob/main/.github/workflows/issue-manager.yml
We should be careful about fixing it: #895 & https://jacobtomlinson.dev/posts/2024/most-stale-bots-are-anti-user-and-anti-contributor-but-they-dont-have-to-be/
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.
Hey, I did not made changes to the flowchart since we had discussed earlier, that flowchart will be reviewed. so that is why a lot of irrelevant info is still up there.
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, that makes sense, thanks for the context! I forgot 🙃
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.
So how do we move forward from here? I know there might some specific suggestions that you may have in your mind. I think we can encompass all of those in a single review, so it becomes easier for me to add them to the PR :)
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 think getting @asteiker 's eyes on this is probably the best way forward. Will you be at next week's hack day? Maybe we can work together to finalize the flowchart.
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.
@mfisher87 @Sherwin-14 Apologies for my radio silence on this recently! Yes I am happy to get back into this and work together at the next hackday. I'll review this in more detail in preparation.
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.
Thanks, Amy!
real == NO ==> intended{Is the intended behaviour?} | ||
intended == YES ==> explain[Explain and close point to docs if needed] | ||
intended == NO ==> open[Keep open for discussion Remove 'pending triage' label] | ||
real == YES ==> real2["1. Remove 'pending triage' label 2. Add related feature label if applicable (e.g. 'feat: ssr') 3. Add priority and meta labels (see 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.
maj == NO ==> p4[p4: important] | ||
unusable == NO ==> workarounds{Are there workarounds for the bug?} | ||
workarounds == NO ==> p3[p3: minor bug] | ||
workarounds == YES ==> p2[p2: edge case has workaround] |
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 have priority levels yet to my knowledge. @asteiker what do you think?
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.
@mfisher87 I really like the idea of priority levels for our bugs. In fact, I have been playing around with this in a new GH project view (see priority column: https://github.com/orgs/nsidc/projects/10/views/5). I don't believe this priority would be visible from the Issue itself unless we add this as a new label. I would suggest we move forward with priorities (since it can really help us identify what is important and what to work on next) through this GH project view and I can take action to update the project board accordingly. Let me know if this sounds like a viable path forward.
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 think that sounds reasonable!
Co-authored-by: Matt Fisher <[email protected]>
Co-authored-by: Matt Fisher <[email protected]>
@mfisher87 I'm sharing some notes from reviewing with @Sherwin-14 during our hackday:
|
If we're seeing users struggling to figure out which template to use, maybe we could do more to guide them in our config? |
@Sherwin-14 how are you feeling about the next steps for this PR? |
I was waiting for the next hackday to start working on this again. I believe collaborating on this alongside @asteiker would be the ideal path forward. We can list all the areas where improvements are needed, in that way I would also get an idea about what steps I need to take moving forward. I had individual reviews from you and Amy, but I feel collaborating together on this coming hackday would make it easier to keep a track of suggestions and improvements. |
@Sherwin-14 Yes, let's finalize these open items together at the hackday. I would also like to start implementing the priority designation in the bug priority project view: https://github.com/orgs/nsidc/projects/10 |
Hey Amy! Could you summarize the improvements that we agreed upon on the last hackday? I do remember some of those but I guess you have a better idea about this. |
This is the initial draft for the triaging document(#754). The pre-commit check is failing because of the use of format argument which seems like a necessity for rendering the doc, more on this here.
📚 Documentation preview 📚: https://earthaccess--901.org.readthedocs.build/en/901/