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 PR template and add info for new contribs #7405

Merged
merged 3 commits into from
Aug 17, 2023

Conversation

maribethb
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Proposed Changes

  • Removes all of the current checkboxes and replaces with a "I validated my changes" checkbox which covers testing, linting, and formatting (in the linked document)
  • Added a new workflow that will comment on the first PR that is opened by a contributor. This lists some helpful resources and instructions.

Reason for Changes

  • Eliminated the "my PR is against develop" and "I branched from develop" because develop is now the default branch in the repo, so contributors would have to go out of their way to make that not true. I don't think we've seen a PR come in against master since we changed the default branch, but we used to (even with the checkbox)
  • Eliminated "I ran format and lint" because we have checks for those. If you didn't run them, the checks will fail.
  • Eliminated "My code conforms to the style guide" because linting should cover that. I did include a link to it in the new contributor message though.

I don't like checking all the boxes, and our team makes the overwhelming majority of PRs. So I prioritized streamlining PR creation, while still trying to provide helpful information to new contributors. Let me know if you think I didn't hit the right balance though.

Documentation

Note: I also updated the style guide and sent a CL to Rachel for review.

Additional Information

I also wrote another GitHub workflow that would simply add a comment if a PR is against master and not develop. I thought this might be useful because I feel like it can be easy to miss which branch is selected for a PR due to it being in small type at the top. I don't always remember to check. I decided against including it here since develop is the default branch, but I can do so if you think it would be useful. It's a really small/straightforward workflow.

@maribethb maribethb requested a review from a team as a code owner August 17, 2023 04:43
@github-actions github-actions bot added the PR: chore General chores (dependencies, typos, etc) label Aug 17, 2023
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
pr-message: >
Welcome! It looks like this is your first pull request in blockly,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: capitalize Blockly

Copy link
Collaborator

@rachel-fenichel rachel-fenichel left a comment

Choose a reason for hiding this comment

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

Approving for after you fix capitalization.

@maribethb maribethb enabled auto-merge (squash) August 17, 2023 18:28
@maribethb maribethb merged commit 0dd814d into google:develop Aug 17, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: chore General chores (dependencies, typos, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants