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

Fix small typo's in PHPDoc #15

Merged
merged 1 commit into from
Nov 29, 2019
Merged

Fix small typo's in PHPDoc #15

merged 1 commit into from
Nov 29, 2019

Conversation

holtkamp
Copy link
Contributor

@holtkamp holtkamp commented Jun 2, 2018

No description provided.

Copy link
Owner

@clue clue left a comment

Choose a reason for hiding this comment

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

@holtkamp Thanks for looking into this and fixing these typos! May I ask you to apply the same corrections to the README.md as well?

@@ -84,7 +84,7 @@ class Queue implements \Countable
* The `$handler` parameter must be a valid callable that accepts your job
* parameters, invokes the appropriate operation and returns a Promise as a
* placeholder for its future result. If the given argument is not a valid
* callable, this method will reject with an `InvalidArgumentExceptionn`
* callable, this method will reject with an `InvalidArgumentException`
Copy link
Owner

Choose a reason for hiding this comment

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

LGTM, the README.md has the exact same typo, can you amend this? :shipit:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, my git skills are still too poor to do a real "amend", so you might need to squash he commits into one....

Copy link
Owner

@clue clue left a comment

Choose a reason for hiding this comment

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

@holtkamp Thank you for the update, the changes LGTM 👍

Can you squash your two commits into a single one?

Squash commits into one with Git
Step 1: choose your starting commit. The first thing to do is to invoke git to start an interactive rebase session: git rebase --interactive HEAD~[N] ...
Step 2: picking and squashing. At this point your editor of choice will pop up, showing the list of commits you want to merge. ...
Step 3: Create the new commit.
https://www.internalpointers.com/post/squash-commits-into-one-git

@holtkamp
Copy link
Contributor Author

holtkamp commented Nov 2, 2019

Github also offers this "squash and merge" at the tip of a button, right?

https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#squash-and-merge-your-pull-request-commits

My experience is that I will fail, retry, fail again and then succeed, which will take another hour of my life... Happy to invest it, but this is a really tiny PR 👼

@clue clue added this to the v1.2.0 milestone Nov 29, 2019
@clue
Copy link
Owner

clue commented Nov 29, 2019

@holtkamp I've reset the branch and force-pushed for you 👍 But you're missing out, you may still want to take a look at interactive rebasing and amending changes for the next time :-)

For the record: I've disabled GitHub's squash function on all my repos because it creates a new squashed commit and leaves PRs as stale branches, meh. Providing proper PRs is much cleaner and allows for a simpler git history IMHO.

Either way, now let's get this in :shipit:

@clue clue merged commit 2192293 into clue:master Nov 29, 2019
@holtkamp
Copy link
Contributor Author

But you're missing out, you may still want to take a look at interactive rebasing and amending changes for the next time :-)

sure, believe me I did, I tried and based on your advise will try again

For the record: I've disabled GitHub's squash function on all my repos because it creates a new squashed commit and leaves PRs as stale branches, meh. Providing proper PRs is much cleaner and allows for a simpler git history IMHO.

Ah, ok, I did not know that / see that, excusez-moi!

@clue
Copy link
Owner

clue commented Nov 29, 2019

@holtkamp No worries, it's a very nuanced difference and your contribution is much appreciated either way! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants