Skip to content
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

chore: update issue and PR templates #3896

Merged
merged 10 commits into from
Apr 5, 2021

Conversation

EisenbergEffect
Copy link
Contributor

Pull Request

📖 Description

Updating our issue and PR templates is the last piece of the work to refresh our GitHub usage approach. We'd like to have a friendly set of templates and guidance for the most common scenarios. This is my take on that based on some prior projects of mine as well as notes I've made over the last few months from team feedback.

Note: While it's technically possible to have multipe PR tempaltes, you can't select them through the UI. The submitter needs to know the template name and add it to the query string, which isn't very user friendly. For this reason, this PR sticks with a single PR template and instead re-organizes the checklists.

🎫 Issues

👩‍💻 Reviewer Notes

This PR (manually) uses the new template, so take a moment to think through how you are experiencing the template right now. A primary characteristic of a good PR is to present an excellent reading experience since the PR will be read by many people, not only now but possibly months or years later. After considering this, have a look at the changed files in this PR. They are all markdown as this is a content-only change. No code is affected. The new templates make use of emojis and commented guidance for each section. What do you think of this approach?

📑 Test Plan

No testing was done as part of this PR other than viewing how files look on GitHub.

✅ Checklist

General

  • I have followed the title guidelines for my PR
  • I have included a change request file using $ yarn change - NOTE: Note implemented yet. Should we leave this off?
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

⏭ Next Steps

This completes the known initial set of work to update our GitHub use for FAST. We'll continue to take feedback and make modification to our templates, labels, issues, PRs, projects, milestones, etc.

Copy link
Contributor

@nicholasrice nicholasrice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just one thought above.

Copy link
Collaborator

@awentzel awentzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the emoji's in the headings? Do they add value? To my eyes, they are a bit distracting and moves my focus away from the text.

@EisenbergEffect EisenbergEffect added the status:blocked Any issue blocked by another label Sep 15, 2020
@EisenbergEffect
Copy link
Contributor Author

This is ready to go but I'm marking this as status:blocked for now. We'll unblock and merge once the new beachball release setup in in place.

@stale
Copy link

stale bot commented Oct 4, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the warning:stale No recent activity within a reasonable amount of time label Oct 4, 2020
@janechu janechu removed the warning:stale No recent activity within a reasonable amount of time label Oct 5, 2020
@janechu
Copy link
Collaborator

janechu commented Oct 5, 2020

Removed stale warning as this is temporarily blocked, not stale.

@@ -0,0 +1,20 @@
---
name: 📚 Documentation
about: Find an inaccuracy in the documentation or something missing?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Find inaccuracies in the documentation or something missing?'

@awentzel
Copy link
Collaborator

@EisenbergEffect per your question regarding yarn change. This feature is in master, so you can include in the PR. We won't use it until we have finalized beachball implementation (it's been tested locally, just not in production scenarios. We need the production scenarios to be fired off manually by @chrisdholt, once that passes I have a quick edit to the CI and it's should run nightly from that point going forward.

@stale
Copy link

stale bot commented Dec 25, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the warning:stale No recent activity within a reasonable amount of time label Dec 25, 2020
@janechu janechu removed the warning:stale No recent activity within a reasonable amount of time label Jan 22, 2021
@chrisdholt
Copy link
Member

@EisenbergEffect I think we are ready to bring this in :)

@EisenbergEffect
Copy link
Contributor Author

@chrisdholt Should I merge now or wait to do it as part of our meeting tomorrow?

@codeclimate
Copy link

codeclimate bot commented Apr 5, 2021

Code Climate has analyzed commit 298679b and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 85.2% (0.0% change).

View more on Code Climate.

@chrisdholt
Copy link
Member

@chrisdholt Should I merge now or wait to do it as part of our meeting tomorrow?

I think you're probably good now to be honest. We can do it tomorrow as well, but I don't see a huge problem with merging prior to tomorrow.

@EisenbergEffect EisenbergEffect merged commit 19c550d into master Apr 5, 2021
@EisenbergEffect EisenbergEffect deleted the users/eisenbergeffect/pr-and-issue-templates branch April 5, 2021 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:blocked Any issue blocked by another
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal] GitHub Usage and Process
6 participants