Skip to content

[Documentation:Developer] Add PR Template Warning#12342

Merged
bmcutler merged 4 commits intomainfrom
pr-warning
Feb 15, 2026
Merged

[Documentation:Developer] Add PR Template Warning#12342
bmcutler merged 4 commits intomainfrom
pr-warning

Conversation

@JManion32
Copy link
Contributor

@JManion32 JManion32 commented Jan 20, 2026 •
edited
Loading

Why is this Change Important & Necessary?

Recently, we have been getting an influx of pull requests that do not comply with Submitty's conventions. This leads to a drawn out review process as we have to wait for contributors to update their PR before we can even review it.

What is the New Behavior?

Added the following to the top of our pull request description template:

<!-- ** Please remove all comment blocks in the description before submitting this PR. ** -->

<!-- NOTE: Please ensure your title and description align with Submitty conventions (see 
https://submitty.org/developer/getting_started/make_a_pull_request for more details). 
Each title has a prefix, and a limit of 40 chars. A description template has been 
provided and must be completed in full. Please also ensure that all CI tests 
are passing. Pull requests that do not meet these requirements are ineligible for 
review and may be closed. -->

Automated Testing & Documentation

We should add more documentation tests that check if comments have been deleted, and verify that the template headers are within the description.

What steps should a reviewer take to reproduce or test the bug or new feature?

Is this change sufficient? Should I add/remove anything?

@github-project-automation github-project-automation bot moved this to Seeking Reviewer in Submitty Development Jan 20, 2026
@codecov
Copy link

codecov bot commented Jan 20, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.67%. Comparing base (35028b0) to head (e432a2f).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #12342   +/-   ##
=========================================
  Coverage     21.67%   21.67%           
  Complexity     9620     9620           
=========================================
  Files           268      268           
  Lines         36164    36164           
  Branches        486      486           
=========================================
  Hits           7837     7837           
  Misses        27845    27845           
  Partials        482      482
Flag Coverage Δ
autograder 21.39% <ø> (ø)
js 2.04% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.69% <ø> (ø)
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 90.72% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Rkoester47
Copy link
Contributor

Rkoester47 commented Jan 20, 2026

I think this is a good move, and personally I agree with your idea of adding a line about PRs that do not comply being closed.

@JManion32
Copy link
Contributor Author

JManion32 commented Jan 20, 2026

I think this is a good move, and personally I agree with your idea of adding a line about PRs that do not comply being closed.

How's this?

Rkoester47
Rkoester47 approved these changes Jan 20, 2026
View reviewed changes
Copy link
Contributor

@Rkoester47 Rkoester47 left a comment

Choose a reason for hiding this comment

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

I think it looks good!

@github-project-automation github-project-automation bot moved this from Seeking Reviewer to Awaiting Maintainer Review in Submitty Development Jan 20, 2026
@IDzyre
Copy link
Member

IDzyre commented Jan 20, 2026

I agree with these comments added to the template.

On a side note, would it be worth adding a CI job that checks the description when the PR is created to see if it contains the template headers ?

@JManion32
Copy link
Contributor Author

JManion32 commented Jan 20, 2026 •
edited
Loading

I agree with these comments added to the template.

On a side note, would it be worth adding a CI job that checks the description when the PR is created to see if it contains the template headers ?

Yes, I think it would be worth it. There are outside cases where some of the headers aren't necessary (like this PR), but the creator can just write "N/A" or something.

@JManion32
Copy link
Contributor Author

JManion32 commented Jan 20, 2026

Also, what do you all think about changing

What steps should a reviewer take to reproduce or test the bug or new feature?

to

What steps should a reviewer take to test this change?

It's a small change that isn't really related to this PR, but I think it sounds better.

@zacharymnp
Copy link
Contributor

zacharymnp commented Jan 23, 2026

Also, what do you all think about changing

What steps should a reviewer take to reproduce or test the bug or new feature?

to

What steps should a reviewer take to test this change?

It's a small change that isn't really related to this PR, but I think it sounds better.

That would be better. You could also change it to something like:

What steps should a reviewer take to reproduce the bug or test the new feature?

@bmcutler bmcutler moved this from Awaiting Maintainer Review to Work in Progress in Submitty Development Jan 29, 2026
@JManion32
Add Delete Comment
ca9e0c0 
1. Delete comment comment
2. Shortened length of lines
3. CI/CD -> CI
@JManion32
Merge branch 'main' into pr-warning
e432a2f
@bmcutler bmcutler merged commit 418de19 into main Feb 15, 2026
25 checks passed
@bmcutler bmcutler deleted the pr-warning branch February 15, 2026 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Rkoester47 Rkoester47 Rkoester47 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Submitty Development
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JManion32 @Rkoester47 @IDzyre @zacharymnp @bmcutler