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

Created pull request template #19

Closed

Conversation

BlackBond06
Copy link

This PR adds a template for creating new pull requests, resolving issue #7.

The following changes were made in this PR:

  • Created a .github folder in the repository's root.
  • Within this folder, created a subdirectory, "PULL_REQUEST_TEMPLATE" to organize the related files.
  • Created a PULL_REQUEST_TEMPLATE.md file which contains the pull request template.

Copy link
Contributor

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Hi @BlackBond06! :) The template is off to a good start! :) I gave some suggestions on how to improve it.
Future tip: Don't do a pull request in a project unless the maintainer has already assigned it to you. It's
open source etiquette

Comment on lines 5 to 7
Please provide a brief description of the change and mention the issue it addresses.Also, include relevant motivation and context. List any dependencies that are required for this change.

Please delete options that are not relevant and please make sure that you X the appropriate boxes.
Copy link
Contributor

Choose a reason for hiding this comment

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

[suggestion] It'd be best to comment out the instructions so that contributors can work find space to add this information.

Suggested change
Please provide a brief description of the change and mention the issue it addresses.Also, include relevant motivation and context. List any dependencies that are required for this change.
Please delete options that are not relevant and please make sure that you X the appropriate boxes.
<!-- Please provide a brief description of the change and mention the issue it addresses.Also, include relevant motivation and context. List any dependencies that are required for this change.
Please delete options that are not relevant and please make sure that you X the appropriate boxes.
-->

Comment on lines 18 to 23
## Screenshots (if applicable)
Include screenshots or GIFs showcasing the changes made, especially if they impact the visual aspects of the app.

## Testing Done

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Also, mention any important details about your testing setup. If you used a testing tool like Jest or Mocha in JavaScript, please let us know.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Screenshots (if applicable)
Include screenshots or GIFs showcasing the changes made, especially if they impact the visual aspects of the app.
## Testing Done
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Also, mention any important details about your testing setup. If you used a testing tool like Jest or Mocha in JavaScript, please let us know.
## Screenshots (if applicable)
<!-- Include screenshots or GIFs showcasing the changes made, especially if they impact the visual aspects of the app.
-->
## Testing Done
<!-- Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Also, mention any important details about your testing setup. If you used a testing tool like Jest or Mocha in JavaScript, please let us know.
-->

Comment on lines 40 to 50
## Checklist

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules
- [ ] I have checked my code and corrected any misspellings
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Checklist
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules
- [ ] I have checked my code and corrected any misspellings
## Checklist
<-- Check all that apply -->
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules
- [ ] I have checked my code and corrected any misspellings

Comment on lines 9 to 10
Fixes # (issue)

Copy link
Contributor

Choose a reason for hiding this comment

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

[suggestion] Consider giving a different heading and instructions on how to link the issue. This would help contributors understand how to do this process.

Suggested change
Fixes # (issue)
## Related Tickets and Documents
<!-- Please use the following format(s) to link your issue(s):
Closes # issue number (e.g, 123)
Relates to # issue number (e.g, 123)
-->

@BlackBond06
Copy link
Author

@CBID2 I'll effect these suggestions immediately. Also, thank you for the tip.

@BlackBond06
Copy link
Author

@CBID2 I've effected the suggested changes in my PR.

Copy link
Contributor

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@ykdojo
Copy link
Owner

ykdojo commented Jan 21, 2024

Did you use any template to create this template?

@BlackBond06
Copy link
Author

Did you use any template to create this template?

Yes, I did.

@ykdojo
Copy link
Owner

ykdojo commented Jan 21, 2024

Let's not use a template here. Chances are, it's too long and some of it is not relevant for us

@CBID2
Copy link
Contributor

CBID2 commented Jan 21, 2024

Let's not use a template here. Chances are, it's too long and some of it is not relevant for us

What do you recommend as an alternative @ykdojo?

@ykdojo
Copy link
Owner

ykdojo commented Jan 21, 2024

It'll be better if we make it from scratch and expand from there.

@BlackBond06
Copy link
Author

So a major issue with this current template is the length ?

@ykdojo
Copy link
Owner

ykdojo commented Jan 21, 2024

Yes that is a major thing with it

@BlackBond06
Copy link
Author

Okay. I'll redraft it to suit the current stage of the project. Thank you for your feedback.

@BlackBond06
Copy link
Author

Hi @ykdojo, I have made corrections to the PR as requested. Please review.

@ykdojo
Copy link
Owner

ykdojo commented Jan 22, 2024

I'm still not sure how this would add value to this project. It includes a mention of a style guide, for example, which we don't have.

@BlackBond06
Copy link
Author

Okay noted. I'll make the change. Are there any other areas in this PR you feel should be changed ?

@ykdojo
Copy link
Owner

ykdojo commented Jan 22, 2024

It should be built from scratch without using a template.

@ykdojo
Copy link
Owner

ykdojo commented Feb 3, 2024

No activity here in two weeks. I'll close it if @BlackBond06 has no updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants