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

feat: added a detailed PR Template #1866

Merged
merged 11 commits into from Oct 30, 2022
Merged

Conversation

MrKrishnaAgarwal
Copy link
Contributor

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Feature, Docs

  • Why was this change needed? (You can also link to an open issue here)

  • This change was needed because I think a detailed PR template would be better and is a good option both for maintainers and contributors

  • Other information:

  • Any suggestions and improvements are welcome!
    cc @scopsy

@MrKrishnaAgarwal
Copy link
Contributor Author

MrKrishnaAgarwal commented Oct 27, 2022

Preview:

@MrKrishnaAgarwal
Copy link
Contributor Author

MrKrishnaAgarwal commented Oct 27, 2022

After the user fills, example:

@MrKrishnaAgarwal
Copy link
Contributor Author

I think it's a good addition, rest up to you :)

@MrKrishnaAgarwal MrKrishnaAgarwal changed the title added a detailed pr template feat: added a detailed PR Template Oct 27, 2022
@davidsoderberg
Copy link
Contributor

Hi @MrKrishnaAgarwal Thanks for the PR :
We have discussed this topic before inside of Novu and our thinking is that eslint, prettier and Github actions should be able to fix/answer the questions you have suggested 😄

What I think would be good to add is maybe some text under the "other information" heading we have today is that we would appreciate if screenshots was provided of UI changes, what do you think @MrKrishnaAgarwal, @ainouzgali @p-fernandez @djabarovgeorge @BiswaViraj ?

@MrKrishnaAgarwal
Copy link
Contributor Author

MrKrishnaAgarwal commented Oct 27, 2022

and I think the UI changes screenshot can be added by the user under the Screenshots section

@davidsoderberg
Copy link
Contributor

Hi @MrKrishnaAgarwal Thanks for the PR :
We have discussed this topic before inside of Novu and our thinking is that eslint, prettier and Github actions should be able to fix/answer the questions you have suggested 😄

Sure, I can add that point there, how's the PR Template overall?

I think most of it is not needed because the stuff mentioned in my first comment but we might add some text in the current "other information" section about screenshots 😄

.github/PULL_REQUEST_TEMPLATE.md Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
Copy link
Contributor

@p-fernandez p-fernandez left a comment

Choose a reason for hiding this comment

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

Please address @davidsoderberg comments.

@MrKrishnaAgarwal
Copy link
Contributor Author

Please address @davidsoderberg comments.

Okay Sure!

@MrKrishnaAgarwal
Copy link
Contributor Author

@davidsoderberg @p-fernandez I have resolved all the things and updated accordingly.
Please review now because I think it is Ready-To-Merge now, any other improvements are welcome!

@MrKrishnaAgarwal MrKrishnaAgarwal requested review from scopsy and removed request for ainouzgali October 29, 2022 10:08
@MrKrishnaAgarwal
Copy link
Contributor Author

@ainouzgali and @scopsy Can you review the PR and merge it!

@ainouzgali ainouzgali merged commit 6927d24 into novuhq:main Oct 30, 2022
@MrKrishnaAgarwal MrKrishnaAgarwal deleted the patch-1 branch October 30, 2022 11:58
@MrKrishnaAgarwal
Copy link
Contributor Author

thankyou @ainouzgali

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

Successfully merging this pull request may close these issues.

None yet

5 participants