Fixing issue #107 #127
Fixing issue #107 #127
Conversation
src/ProxyClient/Nginx.php
Outdated
There was a problem hiding this comment.
why is this not optional anymore?
|
thanks simone. i asked a bunch of questions, but i think the approach looks the best we can do. regarding versioning, is this a bugfix or a new feature? meaning should this go into 1.1 or 1.0? |
There was a problem hiding this comment.
I would name it getHttpClient to avoid confusing as this is inside a ProxyClient
There was a problem hiding this comment.
Due we have setBaseUrl we can change this to getBaseUrl ? No ?
There was a problem hiding this comment.
getBaseUrl would be a URL, but here we return a client object.
|
ping @hpatoio |
1 similar comment
|
ping @hpatoio |
|
Thanks @dbu for pinging ! I'll soon come back to close this ! |
|
@hpatoio @ddeboer do you have an idea what just happened here? this makes no sense to me, github is claiming there are no commits in this pull request, but then saying @ddeboer merged something. looking at that commit it seems to have nothing to do with this PR. @hpatoio can you open a new PR with your branch? |
|
Whoa! Timestamp coincides with my merging of #137. I guess GitHub screwed up here? |
|
i will report this to @github - meanwhile i guess opening a new PR from issue_107 is the way to go, then check if all comments above really have been adressed or if thats part of the bug. |
|
i got an instant reply from them. it looks like @hpatoio destroyed his branch and let it point to the current master. so suddenly there was nothing more to merge anymore... @hpatoio do you still have the code somewhere and can you do a new PR with it? 4d907b8 might be orphaned now, you could try email from github: 3 days ago, user hpatoio force-pushed the head branch of that PR (hpatoio:issue_107) from 4d907b8 to 9073dc9. At that moment, the tip of the head branch (commit 9073dc9) was already in the base branch of the PR (the base is FriendsOfSymfony:master). In other words, the pull request didn't contain any new commits when compared with the base branch. Once something was pushed to the base branch of the PR, we checked open PRs to see if any of them can be marked as merged (e.g. if the pull request was merged manually via the command line). And when that check was run -- it noticed that the PR in question can be marked as merged since it didn't have any new commits when compared to the base branch. In other words, when ddeboer merged #137, the master branch was updated, which triggered the check and also merged the other PR as merged. While it's confusing that ddeboer was listed as the user who merged that other PR, the fact that it was marked as merged is expected behavior for now. Does this perhaps explain the behavior you observed? Let me know if it doesn't and we'll do some more digging. Also, I'll mention this to the team -- again, it's confusing that ddeboer was listed as the user who merged the PR (and I'm sorry if that caused problems for you). |
|
LOL ! Yes, my fault. I was just trying to update my branch but I messed things up ! I still have my code (it wasn't that much) and I'm working on it ! I'll create a new branch and a new PR ! |
4 participants
No description provided.