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

add pr template #633

Merged
merged 8 commits into from
Jul 13, 2019
Merged

Conversation

kendonB
Copy link
Collaborator

@kendonB kendonB commented Jul 4, 2019

This PR uses the proposed template

Description

This pull request adds a template for adding pull requests.

Related Issue

None.

Motivation and Context

Having a template for pull requests helps to add a little discipline to the process and helps to remind contributors what needs to be done before something gets merged.

How Has This Been Tested

I have done no testing as it needs to be merged in order to test with GitHub.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • I have checked that my code does not duplicate functionality elsewhere in Caster.
  • My code implements all the features I wish to merge in this pull request.
  • My change requires a change to the documentation.

Maintainer/Reviewer Checklist

  • Basic functionality has been tested and works as claimed.
  • New documentation is clear and complete.
  • Code is clear and readable.

@LexiconCode LexiconCode added the Meta Everything that is not directly related Code or Documentation, e.g. GitHub, Wiki, Website, Community label Jul 4, 2019
@LexiconCode
Copy link
Member

That would work pretty well for code PR in the traditional sense. However grammars can be viewed a little differently.

Things to think about for a grammar PR.

  • renamed any existing specs? (Typically discouraged if possible without strong rationale.)
  • check and utilize existing command names in the creation of their specs?
  • check to see if they're duplicating functionality that already exists in the current grammar set provided by Caster?

Should we have PR templates for both code PR's and grammar PR's?

@kendonB kendonB mentioned this pull request Jul 4, 2019
10 tasks
@kendonB
Copy link
Collaborator Author

kendonB commented Jul 4, 2019

I don't think we have the nice button interface for PRs like we do for issues. We could achieve that with an extra section that people can delete. I think the benefits are worth a bit of extra trouble

@LexiconCode
Copy link
Member

I don't think we have the nice button interface for PRs like we do for issues. We could achieve that with an extra section that people can delete. I think the benefits are worth a bit of extra trouble

I think that's a good plan for now.

@LexiconCode
Copy link
Member

LexiconCode commented Jul 6, 2019

I think two more checkboxes might be helpful to indicate the status of the pull request. It is hard to know as contributors can't tag (unlike maintainers) the pull requests.

Status

  • WIP
  • Complete

@LexiconCode LexiconCode added the WIP An work in progress label Jul 9, 2019
@kendonB
Copy link
Collaborator Author

kendonB commented Jul 12, 2019

@LexiconCode see if you prefer my latest change to your edit? I incorporated it into the main checklist rather than having it separate and clarified the language a bit.

kendonB@f21436e#diff-aabc15087a0ad90f49f516c89602877b

@kendonB
Copy link
Collaborator Author

kendonB commented Jul 12, 2019

@LexiconCode also review these changes which modifies the pull request template for app grammars as you requested in a concise way.

kendonB@ba3c8f8#diff-aabc15087a0ad90f49f516c89602877b

I also deliberately do not use the language "spec" here as it's particularly confusing to newcomers. I like the terms "command phrases" and "command actions" as it's immediately obvious what these terms mean.

@kendonB kendonB added Needs Review A pull request needs a code review. and removed WIP An work in progress labels Jul 12, 2019
@LexiconCode LexiconCode removed the Needs Review A pull request needs a code review. label Jul 13, 2019
@LexiconCode LexiconCode merged commit bb84c7a into dictation-toolbox:master Jul 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta Everything that is not directly related Code or Documentation, e.g. GitHub, Wiki, Website, Community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants