Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Reorder the checkboxes in the PR template #3666

Merged
merged 1 commit into from
Mar 13, 2021
Merged

Reorder the checkboxes in the PR template #3666

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@

## Pre-launch Checklist

- [ ] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. `[shared_preferences]`
- [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities.
- [ ] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
- [ ] I signed the [CLA].
Copy link
Contributor

Choose a reason for hiding this comment

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

New contributors generally do this after creating the PR since that's when the bot tells them to. I'm not sure whether the ordering is intended to be new or established contributors; I would have thought the former since for established contributors the checklist is much more rote.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well to be frank I put the order in the order that I usually complete them, but in any case, wouldn't the order be the same here for new or established contributors? New ones should hopefully read the contributor guide first, then the other two, before they write their PR, and certainly before they get it reviewed, but they don't need to sign the CLA until it's ready to land, really. And established contributors have already done all four.

- [ ] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. `[shared_preferences]`
- [ ] I listed at least one issue that this PR fixes in the description above.
- [ ] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
- [ ] I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
- [ ] I updated CHANGELOG.md to add a description of the change.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably put these two last.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well until you update the changelog, the tests can't pass, so I figured the tests one had to go last, and then for thematic consistency I put the other test one with it...

Copy link
Contributor

Choose a reason for hiding this comment

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

There's only a failure from not updating the CHANGELOG of you update the version, so as long as these two stay together it won't affect tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Surely the checkbox for "all the tests pass" has to go last though? Since as soon as you make any change the tests have to run again, so you can't know if the tests have passed yet? Seems like by definition you have to check the "changelog" one before the "tests" one...

Anyway I don't feel strongly about this at all, I'm happy to defer to whatever order you'd like to see. Let me know what order you want it in and I'll land that. :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that makes sense. LGTM as-is.

- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I signed the [CLA].
- [ ] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on [Discord].
Expand Down