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: fix spelling error in angular component class description #32971

Closed
wants to merge 1 commit into from

Conversation

ODAVING
Copy link
Contributor

@ODAVING ODAVING commented Oct 2, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@ODAVING ODAVING requested a review from a team as a code owner October 2, 2019 18:29
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@ODAVING
Copy link
Contributor Author

ODAVING commented Oct 2, 2019

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Oct 2, 2019
@ODAVING ODAVING changed the title Fixed spelling error in angular component class description #32948 docs: Fixed spelling error in angular component class description Oct 2, 2019
@ngbot ngbot bot added this to the needsTriage milestone Oct 2, 2019
@ODAVING ODAVING changed the title docs: Fixed spelling error in angular component class description docs: Fixed #32984 spelling error in angular component class description Oct 2, 2019
@ODAVING ODAVING changed the title docs: Fixed #32984 spelling error in angular component class description docs: Fixed #32948 spelling error in angular component class description Oct 2, 2019
@ODAVING ODAVING changed the title docs: Fixed #32948 spelling error in angular component class description docs(quick-reference): Fixed #32948 spelling error in angular component class description Oct 2, 2019
Copy link
Member

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

To be honest, I think this whole sentence doesn't really make sense and that we should just remove the whole thing. What do you think @jbogarthyde ?

@petebacondarwin
Copy link
Member

@ODAVING thanks for submitting this. Please note that the commit message must follow our required syntax: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit

Copy link
Contributor

@jbogarthyde jbogarthyde left a comment

Choose a reason for hiding this comment

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

How about:

In Angular, you create a component class to contain the data model and control methods. Use the TypeScript export keyword to export the class so that the functionality can be imported into NgModules.

@ODAVING
Copy link
Contributor Author

ODAVING commented Oct 7, 2019

@petebacondarwin and @jbogarthyde thank you for reviewing this. I'll make the changes asap.

Copy link
Member

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Great! Can you please squash together the two commits?

@petebacondarwin petebacondarwin added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews target: patch This PR is targeted for the next patch release labels Oct 7, 2019
@ODAVING ODAVING force-pushed the angular#32948 branch 2 times, most recently from facb6c7 to 1899328 Compare October 8, 2019 01:33
@kapunahelewong
Copy link
Contributor

Thanks for this, @ODAVING. Could you rebase and then tag me when you're ready? I'd like to help you move this forward. By the way, I have notes on rebasing if you'd find them helpful. Thank you!

@kapunahelewong kapunahelewong added this to Waiting for OP in docs Oct 18, 2019
@ngbot ngbot bot removed this from the needsTriage milestone Oct 18, 2019
@kapunahelewong
Copy link
Contributor

Hm, I think this might be a flake. Or, wait. Did you remember to push up after rebasing? I'm still seeing your branch as being well behind. https://github.com/ODAVING/angular/tree/angular%2332948

@ODAVING
Copy link
Contributor Author

ODAVING commented Oct 18, 2019

@kapunahelewong I forced pushed after rebasing it 11 days ago. The ci/circleci: test_aio_local_ivy test keeps failing though.

@ODAVING
Copy link
Contributor Author

ODAVING commented Oct 18, 2019

@kapunahelewong The only reason according to circleci_ivy that it's failing is the following message:
"Command 'build-local-with-ivy-ci' not found." Which I don't even know why it would be running it, since it's a simple documentation. Any insight into this would be helpful

@kapunahelewong
Copy link
Contributor

Oh, I thought you meant you'd just rebased today. Could you try it? 🙏🏼

Sometimes there are flakes and a new rebase can help clear them up. I don't think it's anything you've done (since it's referring to Ivy). And if you bring it as up-to-date as possible, that should help. Then if it still fails, either that one or another, it's easier for us to get it green for merge since a whole bunch of other updates are in there. Does that help give context?

@ODAVING
Copy link
Contributor Author

ODAVING commented Oct 18, 2019

@kapunahelewong Done, hoping it works this time 🤞

@kapunahelewong
Copy link
Contributor

Yay! It worked! But, I only just noticed that there are two commits. Could you squash? Here are my own notes on the workflow. You're so close! :)

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@ODAVING
Copy link
Contributor Author

ODAVING commented Oct 18, 2019

When I try to squash, do I have to go through all 260 commits that my branch was behind of, and 'f' them all? @kapunahelewong

@kapunahelewong
Copy link
Contributor

kapunahelewong commented Oct 21, 2019

No, you don't need to go through all those. Just f/fixup the second one. So, here's the way you want to go through it:

git rebase -i HEAD~2

^^That will only give you the option to squash 2 commits. Here's where that command gets me on just some branch I have locally (unrelated to this PR, so disregard the commit messages):

image

Next, I change the second (my more recent commit) to be f:

image

Then I save and exit. Now if I do a git log --oneline, I have only the one commit:

57e6e7c75f (HEAD -> my-test-branch) docs: fix animations reference links to api pages

Now, if I were to push up, that single commit would be the one to show up in my PR. My second looks like it's gone, but its work is still in there.

After you've squashed, the rebase will take care of the 260-ish commits. Those should be handled automatically. Does that help?

EDIT: Oh! I see you rebased fine! Well done! Tag me when you've squashed. 🌟

@ODAVING
Copy link
Contributor Author

ODAVING commented Oct 21, 2019 via email

change component class section of the docs

Closes angular#32948
@kapunahelewong
Copy link
Contributor

Look, @ODAVING! I fixed the extra merge commit! (I've never done that before on someone else's PR). Yay!!!!! Rebased and amended!

@kapunahelewong kapunahelewong changed the title docs(quick-reference): Fixed #32948 spelling error in angular component class description docs: fix spelling error in angular component class description Oct 23, 2019
@kapunahelewong kapunahelewong added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Oct 23, 2019
@ngbot ngbot bot added the action: merge The PR is ready for merge by the caretaker label Oct 23, 2019
Copy link
Contributor

@kapunahelewong kapunahelewong left a comment

Choose a reason for hiding this comment

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

Thank you, @ODAVING!!

Merge assistance: global approval for docs changes. Thank you!

AndrewKushnir pushed a commit that referenced this pull request Oct 23, 2019
change component class section of the docs

Closes #32948

PR Close #32971
@jbogarthyde jbogarthyde moved this from Waiting for OP to Done in docs Oct 29, 2019
@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 Nov 23, 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 cla: yes effort1: hours merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note risk: low target: patch This PR is targeted for the next patch release type: bug/fix
Projects
docs
Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants