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

PR request template #281

Merged
merged 1 commit into from
Aug 24, 2023
Merged

PR request template #281

merged 1 commit into from
Aug 24, 2023

Conversation

annshress
Copy link
Collaborator

@annshress annshress commented Aug 23, 2023

Addresses (eg: #1)

Depends on (eg: #1)

Changes

  • List of changes
  • Breaking changes
  • Changes to configurations

This PR doesn't introduce any:

  • Binary files
  • Temporary files, auto-generated files
  • Secret keys
  • Local debugging print statements
  • Unwanted comments (e.g: # Gets user from environment for code os.environ['user'] )

This PR contains valid:

  • tests

Copy link
Member

@philipmac philipmac left a comment

Choose a reason for hiding this comment

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

How about changing print statements to not useful / local debugging print statements? In some cases simple prints to STDOUT are ok (eg we might redirect STDOUT into a log).

Can we add Binary Files to the top of the list, make the next element of the list be something like Files unrelated to project, eg temp files / log files / other
Make secret keys its own element in the list.
Remove Typos (I don't think is is helpful, people intend not to make typos, they won't see this and remember that they should remove the typos they put in before).
I guess an example of the type of comment that might be unwanted?

@annshress annshress force-pushed the feature/pr_request branch 2 times, most recently from 2a57bc2 to 47921ee Compare August 24, 2023 19:15
@philipmac philipmac merged commit 1922661 into main Aug 24, 2023
2 checks passed
@philipmac philipmac deleted the feature/pr_request branch August 24, 2023 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants