PR template #35
Merged
PR template #35
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Problem - *The template is hardly used by contributors unless they're part of Apps group.* ## Solution - *Add an automatic template for all PR* ## PR Status Could be reviewed and tested. ## Validation --- *Minor decision* - [ ] **Code review** : - [ ] **Approval (LGTM)** : - [ ] **Approval (LGTM)** : When the PR is mark as ready to merge, you have to wait for 3 days before really merge it.
frju365
approved these changes
May 23, 2018
frju365
approved these changes
May 23, 2018
JimboJoe
reviewed
May 23, 2018
pull_request_template.md
Outdated
| - *And how you fix that* | ||
|
|
||
| ## PR Status | ||
| - [ ] Work finished. |
There was a problem hiding this comment.
A very minor wording suggestion: I would interpret "work" as "every checkbox", and would suggest to use "coding" here. What do you think?
There was a problem hiding this comment.
Honestly I think that we can't expect that all the check boxes are going to be validated for each PR.
Myself, I don't test an upgrade each time, now I trust the CI on this point.
And for an minor upgrade without consequences, I'll probably not even try the fix, and again trust the CI.
So, for me, "Work finished" means that the PR is complete. That's, for example, not true for #33.
And, again for #33, that doesn't mean code, but finding the real cause of the issue.
The idea by adding those check boxes is really to know what has been done by the dev who add the PR. And what hasn't.
There was a problem hiding this comment.
OK, then what difference between "work finished" and "can be reviewed and tested"?
There was a problem hiding this comment.
Hum, yes I'm not sure that I can say what would be the difference...
Which it's obviously a problem...
But, there's also the case of our testing release, which are finished, but not obviously opened to review yet.
It can be that. But, obviously, if it's not clear, it's useless !
JimboJoe
approved these changes
May 28, 2018
Josue-T
approved these changes
May 28, 2018
maniackcrudelis
added a commit
to YunoHost-Apps/agendav_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/ampache_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/baikal_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/dokuwiki_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/etherpad_mypads_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/hextris_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/jirafeau_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/kanboard_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/my_webapp_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/nextcloud_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/opensondage_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/phpmyadmin_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/piwigo_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/rainloop_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/roundcube_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/shellinabox_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/strut_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/synapse_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/transmission_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/ttrss_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/wallabag2_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/wordpress_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
maniackcrudelis
added a commit
to YunoHost-Apps/zerobin_ynh
that referenced
this pull request
May 28, 2018
Duplicated from YunoHost-Apps/searx_ynh#35, merged as a micro decision
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Solution
PR Status
Could be reviewed and tested.
Validation
Minor decision
When the PR is mark as ready to merge, you have to wait for 3 days before really merge it.