Skip to content

docs: use Request::is() instead of getMethod()#7137

Merged
samsonasik merged 1 commit intocodeigniter4:developfrom
kenjis:fix-docs-request-getMethod
Jan 17, 2023
Merged

docs: use Request::is() instead of getMethod()#7137
samsonasik merged 1 commit intocodeigniter4:developfrom
kenjis:fix-docs-request-getMethod

Conversation

@kenjis
Copy link
Copy Markdown
Member

@kenjis kenjis commented Jan 17, 2023

Description
Follow-up #6995

  • use Request::is() instead of getMethod()

Checklist:

  • Securely signed commits
  • [] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [] Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the documentation Pull requests for documentation only label Jan 17, 2023
Copy link
Copy Markdown
Member

@lonnieezell lonnieezell left a comment

Choose a reason for hiding this comment

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

I guess I missed when that method was created. That's an unfortunate name since I think it's checking the URI, not the method, but since that's what the method is called, these changes look fine.

@kenjis
Copy link
Copy Markdown
Member Author

kenjis commented Jan 17, 2023

Request::is() checks the type of a request.
https://codeigniter.com/user_guide/incoming/incomingrequest.html#is

It is already overridden by CodeIgniter HTMX.
https://michalsn.github.io/codeigniter-htmx/incoming_request/#is

@lonnieezell
Copy link
Copy Markdown
Member

I wasn't suggesting we change it now. Just bummed I missed that discussion. No worries. :)

@samsonasik samsonasik merged commit f7fcf28 into codeigniter4:develop Jan 17, 2023
@samsonasik
Copy link
Copy Markdown
Member

Thank you @kenjis

@kenjis kenjis deleted the fix-docs-request-getMethod branch January 17, 2023 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Pull requests for documentation only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants