Skip to content

Add getCookieParam method#1928

Closed
martynbiz wants to merge 1 commit intoslimphp:3.xfrom
martynbiz:patch-1
Closed

Add getCookieParam method#1928
martynbiz wants to merge 1 commit intoslimphp:3.xfrom
martynbiz:patch-1

Conversation

@martynbiz
Copy link
Copy Markdown
Contributor

Consistent with getParam and getQueryParam

Consistent with getParam and getQueryParam
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.4%) to 96.246% when pulling 48a8d46 on martynbiz:patch-1 into 805150e on slimphp:3.x.

@dopesong
Copy link
Copy Markdown
Contributor

dopesong commented Jul 5, 2016

Tests missing :)

@AlexDanault
Copy link
Copy Markdown

AlexDanault commented Jul 5, 2016

I'd also add that getCookieParam should be after getCookiesParams and not before, to be consistent with getQueryParam (which is after getQueryParams) and getParsedBodyParam (also after getParsedBodyParams)

@martynbiz
Copy link
Copy Markdown
Contributor Author

martynbiz commented Jul 5, 2016

Hi, thanks for the feedback. Sorry, I'm a little new to creating PR. I hope I've done it correctly here - #1929 - let me know if there is anything I've missed or done incorrectly. Thanks :)

@dopesong
Copy link
Copy Markdown
Contributor

dopesong commented Jul 5, 2016

So you don't need to create new PR, just add new commits to existing branch and PR will be updated automatically. :)

$result = $getCookies[$key];
}
return $result;
}
Copy link
Copy Markdown

@jony4 jony4 Jul 23, 2016

Choose a reason for hiding this comment

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

How about this?

    public function getCookieParam($key)
    {
        $cookies = $this->cookies;
        if (isset($cookies[$key])) {
            return $cookies[$key];
        }
        return false;
    }

@akrabat
Copy link
Copy Markdown
Member

akrabat commented Jul 26, 2016

Replaced by #1929

@akrabat akrabat closed this Jul 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants