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

docs(animations): API doc misspelled word #16554

Merged

Conversation

jasonhodges
Copy link
Contributor

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Other... Please describe: documentation correction

What is the current behavior? (You can also link to an open issue here)
currently here it says at different statrting/ending times

What is the new behavior?
should say at different starting/ending times

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[x] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@gkalpak gkalpak added comp: docs action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels May 5, 2017
@gkalpak gkalpak added this to Content Only in docs-infra May 5, 2017
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

LGTM except for the commit messages (which also break CI). Can you please merge the commits and use animations for subject?

@wardbell wardbell moved this from Content Only to MERGE in docs-infra May 7, 2017
@wardbell wardbell moved this from MERGE to Content Only in docs-infra May 7, 2017
@wardbell
Copy link
Contributor

wardbell commented May 7, 2017

Also overlaps with #16508 which is approved for merge.

For full credit, remove the first commit (covered separately in #16508) and just keep the second. We can merge it then. Right now, we can't merge it because the first commit violates the commit message naming rules enforced by CI.

There is good reason for you confusion

There is a guide called "animations" (plural) and the Angular module is called "animations". By beginning your commit with "docs(animations)", you're telling us that you're making a change to the Angular module API docs. Which you ARE in the second commit.

You're also making a change to the guide documentation. Those kinds of changes should begin "docs(aio): ". In your case it might be something like "docs(aio): Animations guide duplicate word removed".

The two kinds of commits should be in separate PRs. Your guide commit is already in the separate PR #16508 and is good to go. This PR #16554 should be devoted to the Angular module API docs fix.

The reason we separate these is that pure guide fixes don't touch Angular code and are much easier to approve and merge than API docs fixes. Even a simple documentation change that touches code, as this PR #16554 does, has a higher bar to approval.

How the world is going to understand these distinctions is beyond me.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@googlebot googlebot added cla: no and removed cla: yes labels May 7, 2017
@jasonhodges
Copy link
Contributor Author

@wardbell sorry I've gotten these two changes into a git mess. Working on it

@jasonhodges jasonhodges force-pushed the jasonhodges-misspelling_correction branch from 8323339 to a2a8dfb Compare May 7, 2017 21:03
@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels May 7, 2017
@wardbell wardbell added this to Review in docs Jun 21, 2017
@wardbell wardbell removed this from Content Only in docs-infra Jun 21, 2017
@wardbell
Copy link
Contributor

Thanks. Now moving to the right person.

@wardbell wardbell requested a review from matsko June 24, 2017 01:10
@wardbell wardbell removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jun 24, 2017
@wardbell wardbell changed the title docs(animations): correct misspelled word docs(animations): API doc misspelled word Jun 24, 2017
@wardbell wardbell added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jun 24, 2017
@wardbell wardbell added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 24, 2017
@wardbell wardbell moved this from New PR to Merge in docs Jun 24, 2017
@matsko matsko merged commit f365a0f into angular:master Jun 26, 2017
@wardbell wardbell removed this from Merge in docs Jun 27, 2017
@jasonhodges jasonhodges deleted the jasonhodges-misspelling_correction branch May 11, 2019 02:20
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: animations cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants