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

Added header for apps #823

Merged
merged 10 commits into from Nov 1, 2019
Merged

Added header for apps #823

merged 10 commits into from Nov 1, 2019

Conversation

@AAllport
Copy link
Contributor

AAllport commented Oct 31, 2019

AAllport added 2 commits Oct 31, 2019
Copy link
Collaborator

acrobat left a comment

Can you add the line to set the header to each api method instead of the configure method, this way it's easier to remove it when the beta period is over (example). You could make the configure method private and call it in each api method for example. Thanks!

@acrobat acrobat added the needs-work label Nov 1, 2019
@AAllport

This comment has been minimized.

Copy link
Contributor Author

AAllport commented Nov 1, 2019

Certainly!

AAllport added 2 commits Nov 1, 2019
@AAllport AAllport requested a review from acrobat Nov 1, 2019
AAllport added 2 commits Nov 1, 2019
@AAllport

This comment has been minimized.

Copy link
Contributor Author

AAllport commented Nov 1, 2019

@acrobat is there a release schedule?
Unfortunately, my pr on GrahamCampbell/Laravel-GitHub#95 was rejected, so would be nice to know when we can hope to see these changes hit packagist?

@acrobat

This comment has been minimized.

Copy link
Collaborator

acrobat commented Nov 1, 2019

@AAllport There is no planned release schedule, but I'm planning on publishing a new release soon as there are many fixed/features since last release!

@AAllport

This comment has been minimized.

Copy link
Contributor Author

AAllport commented Nov 1, 2019

@AAllport There is no planned release schedule, but I'm planning on publishing a new release soon as there are many fixed/features since last release!

Looking forward to it!

@AAllport

This comment has been minimized.

Copy link
Contributor Author

AAllport commented Nov 1, 2019

Well that's ruddy mysterious

@acrobat

This comment has been minimized.

Copy link
Collaborator

acrobat commented Nov 1, 2019

@AAllport the travis error is not related to these changes but something incorrectly merged on master, I've pushed a fix to the master branch. Can you rebase your branch onto master? Thanks!

lib/Github/Api/Apps.php Outdated Show resolved Hide resolved
@AAllport

This comment has been minimized.

Copy link
Contributor Author

AAllport commented Nov 1, 2019

My git foo isn't quite as good as some be.
If I do a git pull upstream master and it merges, will that suffice (where upstream is this repo)?
IIRC it will merge commit. or did you specifically want a rebase?

@acrobat

This comment has been minimized.

Copy link
Collaborator

acrobat commented Nov 1, 2019

@AAllport A merge is fine! I will squash the commits when merging the pr!

@acrobat
acrobat approved these changes Nov 1, 2019
@acrobat

This comment has been minimized.

Copy link
Collaborator

acrobat commented Nov 1, 2019

Thanks a lot @AAllport! And congrats on your first contribution! 🎉

@acrobat acrobat merged commit 5071f3e into KnpLabs:master Nov 1, 2019
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@acrobat acrobat removed the needs-work label Nov 1, 2019
@AAllport

This comment has been minimized.

Copy link
Contributor Author

AAllport commented Nov 1, 2019

For what it's worth, I believe this is my 1st functional OSS contribution, 10/10 would definitely recommend 😁
I look forward to the release
All the best!

@acrobat

This comment has been minimized.

Copy link
Collaborator

acrobat commented Nov 3, 2019

@AAllport I've published 2.12.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.