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

do not scope creep! #72161

Merged
merged 1 commit into from
Mar 5, 2024
Merged

do not scope creep! #72161

merged 1 commit into from
Mar 5, 2024

Conversation

GuardianDll
Copy link
Member

Summary

None

Purpose of change

It happened

Describe the solution

Add some requirements for PRs in pull request template, visible by anyone who uses it

Describe alternatives you've considered

Additional bot checks to lock 500+ string PRs untill unblocked

@github-actions github-actions bot added Code: Tooling Tooling that is not part of the main game but is part of the repo. astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Mar 4, 2024
@Fris0uman
Copy link
Contributor

No one reads the template, it doesn't hurt to add a reminder but it should be more concise

@anoobindisguise
Copy link
Contributor

Question. Does the bot count lines which are comments? Or empty lines? Both of those are tools which enhance legibility of code and I worry including them in the 500 line limit will encourage people to submit smooshed together, under commented code to try and fit the limit.

@PatrikLundell
Copy link
Contributor

If the bot uses the incompetent git comparison you'll get a lot of "changed" lines when you remove a loop because all the lines within the loop are considered changed because they're white space shifted to have their indentation adjusted.

Also, you get "huge" changes if you move code, or factor common parts out into support functions, so any kind of attempt to organize things gets penalized.

@Maleclypse
Copy link
Member

Thank you for getting to this before me. I can close a tab on my browser now.

@Maleclypse Maleclypse merged commit d58f1f7 into CleverRaven:master Mar 5, 2024
16 of 22 checks passed
@GuardianDll GuardianDll deleted the scope_creep branch March 5, 2024 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Tooling Tooling that is not part of the main game but is part of the repo. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants