Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(CONTRIBUTING.md): Updated commit types. #15340

Merged
merged 1 commit into from
Nov 17, 2016
Merged

Conversation

butla
Copy link
Contributor

@butla butla commented Oct 31, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
docs update

What is the current behavior? (You can also link to an open issue here)
The commit types went out of sync with https://github.com/angular/angular/blob/master/CONTRIBUTING.md#type
What prompted me to do this fix is that there wasn't 100% clear that test fixes (e.g. for flaky tests) should come in the "test" commits.

What is the new behavior (if this is a feature change)?
Commit types are the same as in Angular repo.

Does this PR introduce a breaking change?
No.

Please check if the PR fulfills these requirements

Other information:
Expanded "test" to also mean test fixes, added "build" and "ci".
This is to mirror the documentation in Angular (without JS) repo.

@@ -241,7 +241,9 @@ Must be one of the following:
semi-colons, etc)
* **refactor**: A code change that neither fixes a bug nor adds a feature
* **perf**: A code change that improves performance
* **test**: Adding missing tests
* **test**: Adding missing tests or correcting existing tests
* **build**: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
Copy link
Member

Choose a reason for hiding this comment

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

Now build sounds almost identical to chore. If we decide to bring this over from Angular 2, we should update other types accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I'll update chore

@Narretz
Copy link
Contributor

Narretz commented Nov 1, 2016

Are build and ci really necessary?

@gkalpak
Copy link
Member

gkalpak commented Nov 1, 2016

AFAICT, the idea is to only have the same commit message guidelines across (angular) projects (see #15341). I am not sure I like the idea, though, because different projects (even ones as similar as Angular 1 vs 2) have different needs.

In the name of consistency, we would either end up having a ton of types that are irrelevant to one project or being restricted to too generic types on another.

@butla
Copy link
Contributor Author

butla commented Nov 2, 2016

I've added some more explanations to #15341.
If you're not happy with the changes I can scrap everything except for the mention that "test" is also for fixing tests.

@butla
Copy link
Contributor Author

butla commented Nov 16, 2016

@gkalpak So... any update? Should I just leave the expanded "test" definition?

@Narretz
Copy link
Contributor

Narretz commented Nov 17, 2016

Yes, please. We don't plan to use the additional types currently.

Expanded "test" to also mean test fixes.
Added a link to the help page about closing issues with commit
messages in the section about the footer.
@butla
Copy link
Contributor Author

butla commented Nov 17, 2016

@Narretz Fixed.

@Narretz Narretz merged commit 7d24af1 into angular:master Nov 17, 2016
ellimist pushed a commit to ellimist/angular.js that referenced this pull request Mar 15, 2017
…g issues

Expanded "test" to also mean test fixes.
Added a link to the help page about closing issues with commit
messages in the section about the footer.

Closes angular#15340
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants