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

Pull request template #423

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Conversation

mellis13
Copy link
Contributor

This PR creates a PR template for SmartSim. The PR template is meant to give more structure and consistency to the developer submitting the PR and the developers reviewing the PR.

It is worth noting that only a single PR template was created. That is, a separate PR template was not created for bugs, features, documentation updates, etc. This is because when there is more than one template, there is no way to access the various templates via the web GUI -- it can only be accessed through manipulation of the site URL query parameters (https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/creating-a-pull-request-template-for-your-repository). To make things easier for new contributors, it was decided to stick with one PR template which can be loaded automatically without query parameters.

The proposed template will not be visible in this repository until it is merged. However, I have pushed the changes to my fork's develop branch (https://github.com/mellis13/SmartSim), and as a result, PRs against my fork of SmartSim will show the rendered template.

@mellis13 mellis13 added the area: CI/CD Issues related to continuous integration and deployment label Nov 18, 2023
@mellis13 mellis13 changed the title Template for pull requests Pull request template Nov 18, 2023
Copy link

codecov bot commented Nov 18, 2023

Codecov Report

Merging #423 (2f3cb9f) into develop (15ba145) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #423   +/-   ##
========================================
  Coverage    89.65%   89.65%           
========================================
  Files           59       59           
  Lines         3617     3617           
========================================
  Hits          3243     3243           
  Misses         374      374           

@mellis13 mellis13 changed the title Pull request template [Draft] Pull request template Nov 18, 2023
@mellis13 mellis13 changed the title [Draft] Pull request template Pull request template Nov 18, 2023
@al-rigazzi
Copy link
Collaborator

My take on this is that this PR template should be used only for large merge actions, such as when a large feature branch is merged. This could be something for maintainers to do, so we could actually think of documenting how one manipulates the URL. We would then make sure a SmartSim maintainer uses this PR every time something major changes and is merged.

@billschereriii
Copy link
Contributor

My take is that having a PR template is a good thing, and I like the content that Matt has generated as part of it. The one place I'm not as sure is that I think the checklist would be better moved to the message where a reviewer approves a PR or requests changes. Unfortunately, there isn't a way to make a template for this.

@billschereriii billschereriii removed their assignment Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: CI/CD Issues related to continuous integration and deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants