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.

Sign up for GitHub

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

Jump to bottom

Improve check list #5587

Merged
merged 4 commits into from
Nov 22, 2019
Merged

Improve check list #5587

merged 4 commits into from
Nov 22, 2019

Conversation

koppor
Copy link
Member

@koppor koppor commented Nov 10, 2019 •
edited
Loading

I see that the checklist is not updated on all PRs. It would help me very much if checked items are done and non-checked items are not done. Currently, we have not-checked items interpreted as "we don't care" or "user doesn't know what todo".

We introduced the checklist to improve the quality of PRs. I like it. Just want to have it used more serious. I hope, this update helps.

Example PR is ttps://github.com//pull/5565 - with my comment why I came up with this idea: #5565

@koppor koppor mentioned this pull request Nov 10, 2019
Merged
6 tasks
tobiasdiez
tobiasdiez requested changes Nov 10, 2019
View reviewed changes
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

It depends on what value the check list should give. For me, the primary goal is that as a dev I'm reminded on what I have to do. The second objective is to make the reviewer's life easier.

For this the current practice was quite good. As a dev, I quickly go through the list and check things I've done and consider if I should also do the other things. As a reviewer, I see immediately what was done and what not (and then can decide if further changes are necessary, e.g. add changelog entry).

If now items are removed because they are not done but the PR creator does not deem them to be a todo, then as a reviewer I have to remember which items were there in the beginning.

.github/PULL_REQUEST_TEMPLATE.md Outdated

## Checklist
Copy link
Member

Choose a reason for hiding this comment

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

The body of the PR is always the most important one, thus I don't like the heading here as it puts too much emphasize on something relatively unimportant.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted the thing to be more intuitively separated. Using -- it always felt a bit off. I wondered whether we should add a section ## Description

.github/PULL_REQUEST_TEMPLATE.md Outdated

----
I went through the following checklist and checked all items. All items with `[ ]` are still a TODO. All items checked with `[x]` are done.
Copy link
Member

Choose a reason for hiding this comment

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

This should be a comment, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I wanted to it to be rendered as text so that the user really takes care about the check list.

Copy link
Member

Choose a reason for hiding this comment

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

It's however inconsistent with the other approaches when creating issues and about the description of a PR.

.github/PULL_REQUEST_TEMPLATE.md Outdated

- [ ] Change in CHANGELOG.md described (if applicable)
- [ ] Tests created for changes (if applicable)
- [ ] Manually tested changed features in running JabRef (always required)
- [ ] Screenshots added in PR description (for bigger UI changes)
- [ ] Ensured that [the git commit message is a good one](https://github.com/joelparkerhenderson/git_commit_message)
Copy link
Member

Choose a reason for hiding this comment

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

The git message doesn't really add anything. If I'm proposing a PR than the commits are already down. This can be added as a helpful comment at the end (for the next time).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we should move that to our contribution guideline

@koppor
Copy link
Member Author

koppor commented Nov 10, 2019

I optimize for the first objective here. Especially for PRs opened for four weeks or more. Then, I cannot (timewise) re-iterate on all comments of a PR...

History: git commit message added at #3758.

@koppor koppor changed the title Make checklist more prominent Improve check list Nov 22, 2019
@koppor koppor merged commit 3bd6524 into master Nov 22, 2019
@koppor koppor deleted the improve-checklist branch November 22, 2019 15:21
koppor pushed a commit that referenced this pull request Aug 15, 2021
Squashed 'buildres/csl/csl-styles/' changes from ec4a4c0..4703b5d
1ee45cb 
4703b5d [Frontiers in ...] Remove formatting for volume (#5580)
3a3eb0c Create journal-of-clinical-and-translational-science.csl (#5588)
cf08af8 Create clinical-pulmonary-medicine.csl (#5589)
98985b1 Create cancer-drug-resistance.csl (#5590)
0af101d Create american-journal-of-sociology.csl (#5591)
b24e1f5 Create san-francisco-estuary-and-watershed-science.csl (#5592)
6f9323c revise Manchester UP monograph style (note-bib) (#5587)
e6f982b Delete harper-adams (#5585)
1eef78f Create international-journal-of-clinical-research.csl (#5584)
f28865e OSCOLA.csl: Add publiser info to generic/working paper secondary sources (CSL type "article"). (#5541)
197ceb1 Create system-dynamics-review.csl (#5575)
1d33da2 Create microcirculation.csl (#5577)
d06f625 Create iawa-journal.csl (#5576)
4705585 Update for corrections (#5578)
c1c7852 Create praxis.csl (#5579)
e012054 Create European Review of Agricultural Economics style (#5567)
b2fa912 Create biomarkers.csl (#5582)
1b00ec0 Create journal-of-international-studies.csl (#5574)
79a5d9c Switch MLA to 9th edition (#5561)
011fef3 Update for corrections (#5573)
ff5490c Create zeitschrift-fur-geschichtsdidaktik.csl (#5569)
9093f2f Create etri-journal.csl (#5570)
c9ba757 Create radiation-protection-dosimetry.csl (#5571)
60632bc Create horttechnology.csl (#5572)
e63820b Add a new journal (magnetic resonance in medical sciences) (#5542)
b656885 Create zeitschrift-fur-papyrologie-und-epigraphik.csl (#5548)
25e1eef Update technische-universitat-dresden-linguistik.csl (#5563)
c0c0da4 Create the-american-journal-of-dermatopathology.csl (#5564)
63f7db9 Update journal-of-the-american-association-of-laboratory-animal-scien… (#5566)
ba64225 Update institut-français-darcheologie-orientale-en.csl (#5553)
32e9031 Create institut-francais-darcheologie-orientale-domaine-des-etudes-arabes-en.csl (#5551)
23e2cf5 Create institut-francais-darcheologie-orientale-domaine-des-etudes-arabes.csl (#5550)
3e1a3f3 Update institut-français-darcheologie-orientale.csl (#5552)
176997d Update veterinary-pathology.csl (#5554)
ee035c1 Update mycologia.csl (#5555)
eb4b3a6 Create international-journal-of-automotive-technology.csl (#5557)
3c4e727 Bump addressable from 2.7.0 to 2.8.0 (#5558)
ccb81c8 Create szociologiai-szemle.csl (#5547)
ed9d740 Update brain.csl to ama.csl dependent (#5546)
ab499f2 Create iran-manual-style (#5350)
6e2b13c Update and rename dependent/estuaries-and-coasts.csl to estuaries-and-coasts.csl (#5534)
8441ff9 Create fachhochschule-suedwestfalen.csl (#5459)
8439cf4 Create freie-hochschule-stuttgart.csl (#5466)
076c513 Update midwestern-baptist-theological-seminary.csl (#5420)
350b900 Update veterinary-pathology.csl (#5543)
06261c0 Create United States International Trade Commission.csl (#5508)
7ae505d Add support for blog posts (CSL: post-weblog). (#5536)
93c97ea Update tud bwl marketing style (#5538)
4308791 Create the-chinese-journal-of-international-politics.csl (#5537)
09f3362 Add hanging-indents to infoclio.ch styles (#5540)
d4dc679 Further improvements to norsk-henvisningsstandad-for-rettsvitenskapelige-tekster.csl (#5539)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 4703b5d
Siedlerchr pushed a commit that referenced this pull request Aug 15, 2021
@koppor
[Bot] Update CSL styles (#7985)
2bb3328 
* Squashed 'buildres/csl/csl-styles/' changes from ec4a4c0..4703b5d

4703b5d [Frontiers in ...] Remove formatting for volume (#5580)
3a3eb0c Create journal-of-clinical-and-translational-science.csl (#5588)
cf08af8 Create clinical-pulmonary-medicine.csl (#5589)
98985b1 Create cancer-drug-resistance.csl (#5590)
0af101d Create american-journal-of-sociology.csl (#5591)
b24e1f5 Create san-francisco-estuary-and-watershed-science.csl (#5592)
6f9323c revise Manchester UP monograph style (note-bib) (#5587)
e6f982b Delete harper-adams (#5585)
1eef78f Create international-journal-of-clinical-research.csl (#5584)
f28865e OSCOLA.csl: Add publiser info to generic/working paper secondary sources (CSL type "article"). (#5541)
197ceb1 Create system-dynamics-review.csl (#5575)
1d33da2 Create microcirculation.csl (#5577)
d06f625 Create iawa-journal.csl (#5576)
4705585 Update for corrections (#5578)
c1c7852 Create praxis.csl (#5579)
e012054 Create European Review of Agricultural Economics style (#5567)
b2fa912 Create biomarkers.csl (#5582)
1b00ec0 Create journal-of-international-studies.csl (#5574)
79a5d9c Switch MLA to 9th edition (#5561)
011fef3 Update for corrections (#5573)
ff5490c Create zeitschrift-fur-geschichtsdidaktik.csl (#5569)
9093f2f Create etri-journal.csl (#5570)
c9ba757 Create radiation-protection-dosimetry.csl (#5571)
60632bc Create horttechnology.csl (#5572)
e63820b Add a new journal (magnetic resonance in medical sciences) (#5542)
b656885 Create zeitschrift-fur-papyrologie-und-epigraphik.csl (#5548)
25e1eef Update technische-universitat-dresden-linguistik.csl (#5563)
c0c0da4 Create the-american-journal-of-dermatopathology.csl (#5564)
63f7db9 Update journal-of-the-american-association-of-laboratory-animal-scien… (#5566)
ba64225 Update institut-français-darcheologie-orientale-en.csl (#5553)
32e9031 Create institut-francais-darcheologie-orientale-domaine-des-etudes-arabes-en.csl (#5551)
23e2cf5 Create institut-francais-darcheologie-orientale-domaine-des-etudes-arabes.csl (#5550)
3e1a3f3 Update institut-français-darcheologie-orientale.csl (#5552)
176997d Update veterinary-pathology.csl (#5554)
ee035c1 Update mycologia.csl (#5555)
eb4b3a6 Create international-journal-of-automotive-technology.csl (#5557)
3c4e727 Bump addressable from 2.7.0 to 2.8.0 (#5558)
ccb81c8 Create szociologiai-szemle.csl (#5547)
ed9d740 Update brain.csl to ama.csl dependent (#5546)
ab499f2 Create iran-manual-style (#5350)
6e2b13c Update and rename dependent/estuaries-and-coasts.csl to estuaries-and-coasts.csl (#5534)
8441ff9 Create fachhochschule-suedwestfalen.csl (#5459)
8439cf4 Create freie-hochschule-stuttgart.csl (#5466)
076c513 Update midwestern-baptist-theological-seminary.csl (#5420)
350b900 Update veterinary-pathology.csl (#5543)
06261c0 Create United States International Trade Commission.csl (#5508)
7ae505d Add support for blog posts (CSL: post-weblog). (#5536)
93c97ea Update tud bwl marketing style (#5538)
4308791 Create the-chinese-journal-of-international-politics.csl (#5537)
09f3362 Add hanging-indents to infoclio.ch styles (#5540)
d4dc679 Further improvements to norsk-henvisningsstandad-for-rettsvitenskapelige-tekster.csl (#5539)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 4703b5d

* Squashed 'buildres/csl/csl-locales/' changes from ec6d62a9e7..7a507fc008

7a507fc008 Fix spelling of October in Thai locale

git-subtree-dir: buildres/csl/csl-locales
git-subtree-split: 7a507fc0084edea8376ee5345879ddc2c99829ba

Co-authored-by: github actions <jabrefmail+webfeedback@gmail.com>
koppor pushed a commit that referenced this pull request Sep 1, 2021
Squashed 'buildres/csl/csl-styles/' changes from ec4a4c0..2b37392
03cae10 
2b37392 Create betriebswirtschaftliche-forschung-und-praxis.csl (#5609)
a52989c Update universitat-zu-koln-seminar-fur-abwl-und-finanzierungslehre.csl (#5610)
47c21bf Add historioPLUS (#5611)
baf11b4 Update urad-rs-za-makroekonomske-analize-in-razvoj.csl (#5607)
636affa Create pain-medicine.csl (#5606)
1d060f9 Update sist02.csl (#5568)
54e8904 Update ucl-university-college-apa.csl (#5597)
9c6308e Creation du style cahiers-du-journalisme.csl (#5595)
7dea54b Update universite-du-quebec-a-montreal.csl (#5596)
b110d07 Update journal-of-paleontology.csl (#5594)
9cc3797 Add workaround for software style in ASA (sociology) (#5464)
459dfb7 Update american-journal-of-sociology.csl (#5604)
f73d62f Create council-of-science-editors-brackets.csl (#5593)
bacf673 Remove extra accent (#5598)
55e1d40 Update system-dynamics-review.csl (#5599)
b8262e4 Update independent style for journal-of-cataract-and-refractive-surgery.csl (#5602)
04da306 Create quaderni-ministero-dei-beni-e-delle-attivita-culturali-e-del-t… (#5600)
8144588 Update journal-of-dental-research.csl (#5601)
4703b5d [Frontiers in ...] Remove formatting for volume (#5580)
3a3eb0c Create journal-of-clinical-and-translational-science.csl (#5588)
cf08af8 Create clinical-pulmonary-medicine.csl (#5589)
98985b1 Create cancer-drug-resistance.csl (#5590)
0af101d Create american-journal-of-sociology.csl (#5591)
b24e1f5 Create san-francisco-estuary-and-watershed-science.csl (#5592)
6f9323c revise Manchester UP monograph style (note-bib) (#5587)
e6f982b Delete harper-adams (#5585)
1eef78f Create international-journal-of-clinical-research.csl (#5584)
f28865e OSCOLA.csl: Add publiser info to generic/working paper secondary sources (CSL type "article"). (#5541)
197ceb1 Create system-dynamics-review.csl (#5575)
1d33da2 Create microcirculation.csl (#5577)
d06f625 Create iawa-journal.csl (#5576)
4705585 Update for corrections (#5578)
c1c7852 Create praxis.csl (#5579)
e012054 Create European Review of Agricultural Economics style (#5567)
b2fa912 Create biomarkers.csl (#5582)
1b00ec0 Create journal-of-international-studies.csl (#5574)
79a5d9c Switch MLA to 9th edition (#5561)
011fef3 Update for corrections (#5573)
ff5490c Create zeitschrift-fur-geschichtsdidaktik.csl (#5569)
9093f2f Create etri-journal.csl (#5570)
c9ba757 Create radiation-protection-dosimetry.csl (#5571)
60632bc Create horttechnology.csl (#5572)
e63820b Add a new journal (magnetic resonance in medical sciences) (#5542)
b656885 Create zeitschrift-fur-papyrologie-und-epigraphik.csl (#5548)
25e1eef Update technische-universitat-dresden-linguistik.csl (#5563)
c0c0da4 Create the-american-journal-of-dermatopathology.csl (#5564)
63f7db9 Update journal-of-the-american-association-of-laboratory-animal-scien… (#5566)
ba64225 Update institut-français-darcheologie-orientale-en.csl (#5553)
32e9031 Create institut-francais-darcheologie-orientale-domaine-des-etudes-arabes-en.csl (#5551)
23e2cf5 Create institut-francais-darcheologie-orientale-domaine-des-etudes-arabes.csl (#5550)
3e1a3f3 Update institut-français-darcheologie-orientale.csl (#5552)
176997d Update veterinary-pathology.csl (#5554)
ee035c1 Update mycologia.csl (#5555)
eb4b3a6 Create international-journal-of-automotive-technology.csl (#5557)
3c4e727 Bump addressable from 2.7.0 to 2.8.0 (#5558)
ccb81c8 Create szociologiai-szemle.csl (#5547)
ed9d740 Update brain.csl to ama.csl dependent (#5546)
ab499f2 Create iran-manual-style (#5350)
6e2b13c Update and rename dependent/estuaries-and-coasts.csl to estuaries-and-coasts.csl (#5534)
8441ff9 Create fachhochschule-suedwestfalen.csl (#5459)
8439cf4 Create freie-hochschule-stuttgart.csl (#5466)
076c513 Update midwestern-baptist-theological-seminary.csl (#5420)
350b900 Update veterinary-pathology.csl (#5543)
06261c0 Create United States International Trade Commission.csl (#5508)
7ae505d Add support for blog posts (CSL: post-weblog). (#5536)
93c97ea Update tud bwl marketing style (#5538)
4308791 Create the-chinese-journal-of-international-politics.csl (#5537)
09f3362 Add hanging-indents to infoclio.ch styles (#5540)
d4dc679 Further improvements to norsk-henvisningsstandad-for-rettsvitenskapelige-tekster.csl (#5539)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 2b37392
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobiasdiez tobiasdiez tobiasdiez requested changes

@LinusDietz LinusDietz LinusDietz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@koppor @LinusDietz @tobiasdiez