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

Create pull_request_template.md #11530

Merged
merged 6 commits into from
Mar 4, 2021
Merged

Conversation

assuntad23
Copy link
Contributor

@assuntad23 assuntad23 commented Mar 2, 2021

What did you do?

  • created pull_request_template.md according to contributing guidelines

Why did you make this change?

(Cite Issue number OR provide rationalization of changes if no issue exists)
(If fixing a bug, please add any relevant error or traceback)
No issue # exists. This PR is to more effectively enforce contributing guidelines located at https://github.com/galaxyproject/galaxy/blob/dev/CONTRIBUTING.md

How to test the changes?

(select the most appropriate option; if the latter, provide steps for testing below)

  1. This PR uses the PR message suggested.

For UI Components

  • I've included a screenshot of the changes

Proposed PR Template based on Contributing Guidelines
@github-actions github-actions bot added this to the 21.05 milestone Mar 2, 2021
pull_request_template.md Outdated Show resolved Hide resolved
@dannon
Copy link
Member

dannon commented Mar 3, 2021

This is great, thanks for following up on this. Intuitively I like reading "I did X to address Y" (so, what before why, instead of "why I'm making a PR; what I did"), but I know that's subjective and this order is fine too.

What about requesting screenshots for stuff that reasonably touches the UI? I find screenshots/gifs incredibly useful both for at-a-glance change review and for finding the changes in question in the UI for more niche stuff.

pull_request_template.md Outdated Show resolved Hide resolved
@jdavcs jdavcs added the procedure/discussion Would be a good candidate for discussion at dev roundtable label Mar 3, 2021
@assuntad23 assuntad23 closed this Mar 3, 2021
@assuntad23 assuntad23 reopened this Mar 3, 2021
@assuntad23
Copy link
Contributor Author

assuntad23 commented Mar 3, 2021

@dannon I followed the same order the Contributing Guidelines outline. I'm not opposed to changing it, by any means.
I agree that the order feels a little bit foreign, but also understand that the thought process might have been the most important information in a PR is probably the issue number.

I'm open to whatever order seems the most appealing to the majority, of course.

As for screenshots, that sounds reasonable as well. I believe @davelopez said that screenshots would have been useful for the testing team, and I whole-heartedly agree, but have just one reservation: I'm hesitant to ask too much of contributors with this change. Do you feel a screenshot would fall under that category?

@hexylena
Copy link
Member

hexylena commented Mar 3, 2021

Please, please request screenshots. There are so many UI contributions that I might want to highlight in user facing changes but because the PR only has a textual description I have to boot up galaxy to check out what was changed, and then look at the code sometimes to guess where it might be visible.

I'd love GIFs where possible but that might be a bigger burden, screenshots feel like a low threshold to cross, everyone has one built in after all

@dannon
Copy link
Member

dannon commented Mar 3, 2021

@assuntad23 Yeah, share the general concern about making this beneficial but not cumbersome but I don't think screenshots are a big ask, and are super helpful.

@jmchilton
Copy link
Member

Might preference would be to merge the last two sections, with all the same stipulations Dannon made above in regards to ordering about this being a subjective preference and the current template being totally fine also.

How to test the changes?

  • I've included appropriate automated tests
  • Instructions for manual testing is as follows:
    1. [add testing steps and prerequisites here if you wrote automated tests covering all your changes]

pull_request_template.md Outdated Show resolved Hide resolved
@dannon
Copy link
Member

dannon commented Mar 4, 2021

I think this is a great start; let's give it a shot and iterate. Thanks @assuntad23

@dannon dannon merged commit 3957655 into galaxyproject:dev Mar 4, 2021
@github-actions
Copy link

github-actions bot commented Mar 4, 2021

This PR was merged without a "kind/" label, please correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation kind/enhancement procedure/discussion Would be a good candidate for discussion at dev roundtable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants